From 9f44625632278557450ff7f884c36020d1c7398c Mon Sep 17 00:00:00 2001 From: kivtxs <131401183+kivtxs@users.noreply.github.com> Date: Mon, 5 Oct 2026 16:37:00 -0400 Subject: [PATCH 1/2] fix(protocol,mls): report a key package refused by this device's clock A peer's key package is valid from an hour before it was minted, judged by the receiver's clock. Between two devices whose clocks disagree by more than that, no session forms and messages and connection requests stay pending forever, and the only trace was a debug line. Two Android phones that had never been online (one set to February 2024, one to October 2025) reproduced it during the v0.28 smoke test. - MlsError::KeyPackageOutsideValidityWindow, from OpenMLS's KeyPackageVerifyError::InvalidLifetime, on every route that admits a peer's package (import, cache read, add_group_member) through one helper. - SecurityWarningCode::KeyPackageOutsideValidityWindow (KEY_PACKAGE_OUTSIDE_VALIDITY_WINDOW), raised by establish_secure_session once per peer through the control-gate throttle: the package has not proved its sender when its window is checked. - The window itself is unchanged. --- CHANGELOG.md | 14 ++ bindings/react-native/src/types.ts | 16 ++- crates/offline-protocol-mls/src/error.rs | 20 +++ crates/offline-protocol-mls/src/manager.rs | 126 ++++++++++++++++-- crates/offline-protocol/src/events.rs | 24 +++- crates/offline-protocol/src/protocol/mod.rs | 25 +++- .../offline-protocol/src/protocol/security.rs | 4 + .../src/protocol/tests/mod.rs | 65 +++++++++ docs/security/threat-model.md | 7 +- docs/spec/local-api.md | 2 +- 10 files changed, 287 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 37d3dc444..6d15a02f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -345,6 +345,20 @@ archived by series under [docs/changelog/](docs/changelog/); see the ### Fixed +- **A key package refused by this device's clock is reported.** A peer's key + package is valid from an hour before it was minted until 30 days after, + judged by the receiver's clock. A device more than an hour behind a peer + cannot start a session with it, and past 30 days neither side can, so no + session forms and messages and connection requests stay pending forever. + The only trace was a debug line. Two Android phones that had never been + online (one set to 2024, the other to 2025) reproduced it. The refusal + now raises a `security_warning` with the new code + `KEY_PACKAGE_OUTSIDE_VALIDITY_WINDOW`, once per peer, and is logged at + `warn`; `MlsError::KeyPackageOutsideValidityWindow` replaces the + `InvalidKeyPackage` text for this one refusal on every route that admits a + peer's package. The window itself is unchanged. TypeScript's + `SecurityWarningCode` union gains the code, so an exhaustive `switch` over it + needs a new arm. - **An old message no longer comes back as a push notification, again and again.** A direct message the relay had pushed stayed in the outbox well past a week, because each probe re-send refreshed its lifetime and so did each diff --git a/bindings/react-native/src/types.ts b/bindings/react-native/src/types.ts index 4fae8d07a..2f7c3b5a6 100644 --- a/bindings/react-native/src/types.ts +++ b/bindings/react-native/src/types.ts @@ -2518,7 +2518,8 @@ export type SecurityWarningCode = | 'GROUP_LEAF_IDENTITY_UNPROVEN' | 'STALE_CONTROL_FRAME' | 'GATEWAY_ADDRESS_BINDING_MISMATCH' - | 'GATEWAY_ADDRESS_DECLARATION_REFUSED'; + | 'GATEWAY_ADDRESS_DECLARATION_REFUSED' + | 'KEY_PACKAGE_OUTSIDE_VALIDITY_WINDOW'; /** * A security-relevant anomaly was detected for a peer. @@ -2597,6 +2598,19 @@ export type SecurityWarningCode = * frames or running a broken build. Nothing is torn down — the frame is * dropped, unacknowledged, and a peer whose frame was genuinely just slow * re-sends a fresh one. + * + * `KEY_PACKAGE_OUTSIDE_VALIDITY_WINDOW` is the same kind of finding for key + * packages, and again the first thing to check is a clock. A peer's key + * package is valid from an hour before it was made until 30 days after, + * judged by this device's clock. A device more than an hour behind a peer + * cannot start a session with it (the peer still can, and usually does). + * Past 30 days each side refuses the other's package, both report this, and + * no encrypted session forms: messages and connection requests wait until + * the clocks agree. This is what happens between phones that have not been + * online to set their time. Many `peer_id`s is this device's clock; one is that + * peer's. Reported once per peer, and `peer_id` is a claim, since the package + * has not proved its sender when its window is checked. Show the user a + * prompt to check the date and time. */ export interface SecurityWarningEvent extends BaseEvent { type: 'security_warning'; diff --git a/crates/offline-protocol-mls/src/error.rs b/crates/offline-protocol-mls/src/error.rs index a76811363..829301662 100644 --- a/crates/offline-protocol-mls/src/error.rs +++ b/crates/offline-protocol-mls/src/error.rs @@ -73,6 +73,23 @@ pub enum MlsError { #[error("Invalid key package: {0}")] InvalidKeyPackage(String), + /// A peer's key package is well formed, but this device's clock falls + /// outside its validity window: before its `not_before`, or after its + /// `not_after`. + /// + /// Split from [`Self::InvalidKeyPackage`] because it has a different + /// remedy, and that remedy is usually a clock rather than the peer. The + /// window is judged against this device's own time, and a package starts + /// only an hour before it was minted, so a device whose clock runs more + /// than an hour behind its peer's refuses every package that peer sends. + /// Past the 30-day lifetime the peer refuses this device's packages too, + /// and no session forms from either side. Reported to the application as + /// `KEY_PACKAGE_OUTSIDE_VALIDITY_WINDOW`; before that it was a debug line, + /// and two phones a year apart sat on a pending connection request with + /// nothing anywhere to say why. + #[error("Key package is outside its validity window by this device's clock")] + KeyPackageOutsideValidityWindow, + /// Failed to create a group. #[error("Group creation failed: {0}")] GroupCreation(String), @@ -507,6 +524,9 @@ impl MlsError { Self::CredentialCreation(_) => "MLS credential creation failed", Self::KeyPackageCreation(_) => "Key package creation failed", Self::InvalidKeyPackage(_) => "The peer's key package was malformed or unusable", + Self::KeyPackageOutsideValidityWindow => { + "The peer's key package is not valid at this device's time" + } Self::GroupCreation(_) => "MLS group creation failed", Self::GroupNotFound(_) => "No local MLS group state for the requested conversation", Self::AddMember(_) => "Adding a member to the MLS group failed", diff --git a/crates/offline-protocol-mls/src/manager.rs b/crates/offline-protocol-mls/src/manager.rs index 77a9fd3bf..b2bb87f7e 100644 --- a/crates/offline-protocol-mls/src/manager.rs +++ b/crates/offline-protocol-mls/src/manager.rs @@ -675,9 +675,7 @@ impl MlsManager { let key_package_in = KeyPackageIn::tls_deserialize_exact(key_package_data) .map_err(|e| MlsError::InvalidKeyPackage(e.to_string()))?; - let key_package = key_package_in - .validate(self.provider.crypto(), ProtocolVersion::Mls10) - .map_err(|e| MlsError::InvalidKeyPackage(e.to_string()))?; + let key_package = self.validate_peer_key_package(key_package_in)?; Self::verify_credential_identity(&key_package, user_id)?; Self::verify_address_binding(&key_package, user_id)?; @@ -702,9 +700,7 @@ impl MlsManager { .map_err(|e| MlsError::InvalidKeyPackage(e.to_string()))?; // Validate using the crypto backend - let key_package = key_package_in - .validate(self.provider.crypto(), ProtocolVersion::Mls10) - .map_err(|e| MlsError::InvalidKeyPackage(e.to_string()))?; + let key_package = self.validate_peer_key_package(key_package_in)?; // Defense in depth: entries written to storage out-of-band must not // come back attributed to the wrong user. Both checks run again here @@ -718,6 +714,23 @@ impl MlsManager { Ok(key_package) } + /// Validates a key package this install did not mint. + /// + /// The one place a peer's package meets OpenMLS's checks, so that a + /// package refused for its validity window comes back as + /// [`MlsError::KeyPackageOutsideValidityWindow`] on every route that + /// admits one (import, the cache read, adding a group member) rather than + /// as free text on some of them. The window is the only refusal whose + /// usual cause is a clock, and the caller reports it as such. + fn validate_peer_key_package(&self, key_package_in: KeyPackageIn) -> Result { + key_package_in + .validate(self.provider.crypto(), ProtocolVersion::Mls10) + .map_err(|e| match e { + KeyPackageVerifyError::InvalidLifetime => MlsError::KeyPackageOutsideValidityWindow, + other => MlsError::InvalidKeyPackage(other.to_string()), + }) + } + /// Requires a key package's validity window to be no wider than this /// install admits. /// @@ -1104,10 +1117,10 @@ impl MlsManager { invitee_user_id: &str, member_key_package: &[u8], ) -> Result<(WelcomeMessage, EncryptedMessage)> { - let key_package = KeyPackageIn::tls_deserialize_exact(member_key_package) - .map_err(|e| MlsError::InvalidKeyPackage(e.to_string()))? - .validate(self.provider.crypto(), ProtocolVersion::Mls10) - .map_err(|e| MlsError::InvalidKeyPackage(e.to_string()))?; + let key_package = self.validate_peer_key_package( + KeyPackageIn::tls_deserialize_exact(member_key_package) + .map_err(|e| MlsError::InvalidKeyPackage(e.to_string()))?, + )?; Self::verify_credential_identity(&key_package, invitee_user_id)?; Self::verify_address_binding(&key_package, invitee_user_id)?; @@ -2041,6 +2054,37 @@ impl MlsManager { .tls_serialize_detached() .map_err(|e| MlsError::Serialization(e.to_string())) } + + /// Mints a key package claiming `claimed` whose validity window runs from + /// `not_before` to `not_after` (seconds since the epoch): what a peer + /// whose clock is somewhere else entirely would send. + /// + /// The window is checked before the identity binding, so the claimed + /// identity is never proved here, which is also what makes the refusal + /// it provokes attacker-reachable. + pub fn key_package_with_window_for_testing( + claimed: &str, + not_before: u64, + not_after: u64, + ) -> Result> { + let storage: Arc = Arc::new(crate::storage::InMemoryStorage::new()); + let provider = MlsProvider::new(MlsStorageAdapter::new(storage)); + let keys = SignatureKeyPair::new(DEFAULT_CIPHERSUITE.signature_algorithm()) + .map_err(|e| MlsError::CryptoGeneration(format!("{:?}", e)))?; + keys.store(provider.storage()) + .map_err(|e| MlsError::CryptoGeneration(format!("storing key: {:?}", e)))?; + let credential = CredentialWithKey { + credential: Credential::new(CredentialType::Basic, claimed.as_bytes().to_vec()), + signature_key: keys.public().into(), + }; + KeyPackage::builder() + .key_package_lifetime(Lifetime::init(not_before, not_after)) + .build(DEFAULT_CIPHERSUITE, &provider, &keys, credential) + .map_err(|e| MlsError::KeyPackageCreation(e.to_string()))? + .key_package() + .tls_serialize_detached() + .map_err(|e| MlsError::Serialization(e.to_string())) + } } /// Returns `true` when `bytes` parse as a well-formed MLS wire message @@ -4889,6 +4933,68 @@ mod tests { .as_secs() } + /// A peer whose clock runs a year ahead of ours mints a window that has + /// not started yet by our clock. It is refused, and refused as a window + /// problem rather than as a malformed package, because that is what tells + /// an application to look at a clock. Two phones a year apart reproduced + /// this, and the only trace was a debug line. + #[test] + fn a_window_that_has_not_started_by_our_clock_is_reported_as_such() { + let (manager, _) = create_addressed_manager("alice"); + let year = 365 * 24 * 60 * 60; + let (peer, bytes) = key_package_with_window( + "bob", + Lifetime::init( + now_secs() + year - 3600, + now_secs() + year + 30 * 24 * 60 * 60, + ), + ); + + let err = manager + .import_key_package(&peer, &bytes) + .expect_err("a window that starts in a year is not valid now"); + assert!( + matches!(err, MlsError::KeyPackageOutsideValidityWindow), + "refused for the wrong reason: {err:?}" + ); + } + + /// The other side of the same skew: a peer whose clock runs behind mints a + /// window that has already closed by ours. + #[test] + fn a_window_that_closed_by_our_clock_is_reported_as_such() { + let (manager, _) = create_addressed_manager("alice"); + let day = 24 * 60 * 60; + let (peer, bytes) = key_package_with_window( + "bob", + Lifetime::init(now_secs() - 60 * day, now_secs() - 30 * day), + ); + + let err = manager + .import_key_package(&peer, &bytes) + .expect_err("a window that closed a month ago is not valid now"); + assert!( + matches!(err, MlsError::KeyPackageOutsideValidityWindow), + "refused for the wrong reason: {err:?}" + ); + } + + /// Every other refusal keeps its old type and text: only the window moved. + /// The width cap in particular stays `InvalidKeyPackage`, because a package + /// claiming a year is a peer's policy, not anyone's clock. + #[test] + fn only_the_window_refusal_changes_type() { + let (manager, _) = create_addressed_manager("alice"); + let (peer, bytes) = key_package_with_window("bob", Lifetime::new(365 * 24 * 60 * 60)); + let err = manager.import_key_package(&peer, &bytes).unwrap_err(); + assert!(matches!(&err, MlsError::InvalidKeyPackage(m) if m.contains("wider than"))); + + let err = manager + .import_key_package(&peer, b"not a key package") + .unwrap_err(); + assert!(matches!(err, MlsError::InvalidKeyPackage(_)), "{err:?}"); + } + /// The case the issue was filed on: mls-rs hands out a year by default, and /// OpenMLS validation admits it because it never applies its own cap. #[test] diff --git a/crates/offline-protocol/src/events.rs b/crates/offline-protocol/src/events.rs index 4337b647b..b08b700c1 100644 --- a/crates/offline-protocol/src/events.rs +++ b/crates/offline-protocol/src/events.rs @@ -331,6 +331,25 @@ pub enum SecurityWarningCode { /// carrier available, and this warning explains a transport that stays /// down while its socket connects fine. GatewayAddressDeclarationRefused, + /// A peer's key package was refused because this device's clock is + /// outside its validity window: the window has not started yet, or has + /// already closed. + /// + /// Like [`Self::StaleControlFrame`], the first thing to check is a clock, + /// and it is usually this device's. A package is valid from an hour before + /// it was minted until 30 days after, by the receiver's clock. So a device + /// more than an hour behind a peer cannot start a session with it, though + /// the peer, whose clock accepts this device's package, still can. Once + /// the gap passes 30 days each side refuses the other's package and no + /// session forms at all: messages and connection requests wait for as long + /// as the clocks disagree, and both sides report this. Phones that have not + /// been online are where this happens. Many different peers is this + /// device's clock; one peer while others are fine is that peer's. + /// + /// The package has not proved its sender when the window is checked, so + /// the peer named is a claim, and the event is reported at most once per + /// peer. + KeyPackageOutsideValidityWindow, } impl SecurityWarningCode { @@ -354,6 +373,7 @@ impl SecurityWarningCode { Self::StaleControlFrame => "STALE_CONTROL_FRAME", Self::GatewayAddressBindingMismatch => "GATEWAY_ADDRESS_BINDING_MISMATCH", Self::GatewayAddressDeclarationRefused => "GATEWAY_ADDRESS_DECLARATION_REFUSED", + Self::KeyPackageOutsideValidityWindow => "KEY_PACKAGE_OUTSIDE_VALIDITY_WINDOW", } } } @@ -4234,6 +4254,7 @@ mod tests { SecurityWarningCode::StaleControlFrame, SecurityWarningCode::GatewayAddressBindingMismatch, SecurityWarningCode::GatewayAddressDeclarationRefused, + SecurityWarningCode::KeyPackageOutsideValidityWindow, ]; for code in all { // serde renders a unit enum variant as a quoted JSON string. @@ -4263,7 +4284,8 @@ mod tests { | SecurityWarningCode::GroupLeafIdentityUnproven | SecurityWarningCode::StaleControlFrame | SecurityWarningCode::GatewayAddressBindingMismatch - | SecurityWarningCode::GatewayAddressDeclarationRefused => {} + | SecurityWarningCode::GatewayAddressDeclarationRefused + | SecurityWarningCode::KeyPackageOutsideValidityWindow => {} } } } diff --git a/crates/offline-protocol/src/protocol/mod.rs b/crates/offline-protocol/src/protocol/mod.rs index 041abaaed..c1c49a698 100644 --- a/crates/offline-protocol/src/protocol/mod.rs +++ b/crates/offline-protocol/src/protocol/mod.rs @@ -2971,12 +2971,33 @@ impl OfflineProtocol { self.pending_key_packages.remove(peer_id); self.delete_peer_key_package_from_storage(peer_id); } else { - { + let imported = { let manager = mls .read() .map_err(|_| Error::Other("MLS lock poisoned".to_string()))?; - manager.import_key_package(peer_id, &received_pkg.key_package_data)?; + manager.import_key_package(peer_id, &received_pkg.key_package_data) + }; + if let Err(offline_protocol_mls::MlsError::KeyPackageOutsideValidityWindow) = + &imported + { + // The usual cause is a clock, on one side or the other, and + // it blocks every session with this peer for as long as the + // clocks disagree. Logged and reported, because this was a + // debug line and nothing else. + warn!( + peer_id = %peer_id, + "Peer key package is outside its validity window by this \ + device's clock; this device cannot start a session with the \ + peer until the clocks agree" + ); + self.warn_control_gate_rejection( + peer_id, + crate::events::SecurityWarningCode::KeyPackageOutsideValidityWindow, + "The peer's key package is not valid at this device's time. \ + Check this device's date and time, then the peer's.", + ); } + imported?; // Create session and get welcome message let welcome = { diff --git a/crates/offline-protocol/src/protocol/security.rs b/crates/offline-protocol/src/protocol/security.rs index f57dfc398..835974a8f 100644 --- a/crates/offline-protocol/src/protocol/security.rs +++ b/crates/offline-protocol/src/protocol/security.rs @@ -899,6 +899,10 @@ impl OfflineProtocol { SecurityWarningCode::UnsignedControlRejected => 1 << 2, SecurityWarningCode::SenderAddressMismatch => 1 << 3, SecurityWarningCode::StaleControlFrame => 1 << 4, + // Not a gate rejection, but the same exposure: the key package has + // not proved its sender when its window is checked, so the peer id + // is attacker-chosen and needs the same once-per-peer bound. + SecurityWarningCode::KeyPackageOutsideValidityWindow => 1 << 5, _ => 0, } } diff --git a/crates/offline-protocol/src/protocol/tests/mod.rs b/crates/offline-protocol/src/protocol/tests/mod.rs index 330b78bd7..b10cc49bf 100644 --- a/crates/offline-protocol/src/protocol/tests/mod.rs +++ b/crates/offline-protocol/src/protocol/tests/mod.rs @@ -40217,6 +40217,71 @@ fn the_ratchet_survives_a_restart() { ); } +/// A peer's key package whose window this device's clock is outside of +/// blocks every session with that peer, and used to say so only in a debug +/// line: two offline phones a year apart sat on a pending connection request +/// with nothing to tell either user to check the date. It is now reported +/// under its own code, once per peer however often the session is retried. +#[test] +fn a_key_package_outside_its_window_by_our_clock_is_reported_once_per_peer() { + let mut alice = protocol_with_mls("alice"); + let warnings: Arc>> = Arc::new(Mutex::new(Vec::new())); + { + let sink = Arc::clone(&warnings); + alice.on_event(move |e| { + if let Event::SecurityWarning { + peer_id, + reason_code, + .. + } = e + { + sink.lock().unwrap().push((peer_id, reason_code)); + } + }); + } + + // Bob's clock runs a year ahead of Alice's. + let now = Utc::now().timestamp() as u64; + let year = 365 * 24 * 3600; + let bob = id("bob"); + let bytes = offline_protocol_mls::MlsManager::key_package_with_window_for_testing( + &bob, + now + year - 3600, + now + year + 30 * 24 * 3600, + ) + .unwrap(); + alice.pending_key_packages.insert( + bob.clone(), + ReceivedKeyPackage { + key_package_data: bytes, + local_expires_at_ms: u64::MAX, + }, + ); + + for _ in 0..3 { + let err = alice + .establish_secure_session(&bob) + .expect_err("no session can form from a package that is not valid yet"); + assert!( + matches!( + err, + Error::Mls(offline_protocol_mls::MlsError::KeyPackageOutsideValidityWindow) + ), + "refused for the wrong reason: {err:?}" + ); + } + + let seen = warnings.lock().unwrap().clone(); + assert_eq!( + seen, + vec![( + bob.clone(), + SecurityWarningCode::KeyPackageOutsideValidityWindow + )], + "exactly one warning naming the peer, whatever the retry count" + ); +} + /// The refusal is reported under its own code, so an integrator can tell a /// clock fault from the signature failures it would otherwise be filed under. #[test] diff --git a/docs/security/threat-model.md b/docs/security/threat-model.md index d93b39ec3..4eae0517f 100644 --- a/docs/security/threat-model.md +++ b/docs/security/threat-model.md @@ -393,7 +393,12 @@ takes its own control plane down, key package exchange included. **Mitigation:** the refusal is reported under its own `STALE_CONTROL_FRAME` code so that a run of them across many peers reads as a local fault rather than an attack, and `security.control_freshness_enforced` turns enforcement off without -a new binary. A leaf node is required to have a time source at pairing for this +a new binary. A key package's validity window is judged the same way and fails +the same way: it runs from an hour before minting to 30 days after, so a device +more than an hour behind its peer cannot start a session (the peer still can), +and past 30 days neither side can and no session forms. The refusal is +reported under `KEY_PACKAGE_OUTSIDE_VALIDITY_WINDOW`, once per peer, and the +window itself is not relaxed. A leaf node is required to have a time source at pairing for this among other reasons; see [leaf provisioning](../spec/leaf-provisioning.md). diff --git a/docs/spec/local-api.md b/docs/spec/local-api.md index 45beb92a2..926008fa8 100644 --- a/docs/spec/local-api.md +++ b/docs/spec/local-api.md @@ -716,7 +716,7 @@ server-side rule an implementation MAY add, and the reference server does. | `DorsReasonCode` | `INITIAL_SELECTION`, `PRIMARY_SELECTED`, `PRIMARY_SUCCESS`, `FALLBACK_SUCCESS`, `ESCALATION_APPLIED`, `CURRENT_UNAVAILABLE` | | `DorsEscalationPhase` | `TRIGGERED`, `APPLIED` | | `DorsEscalationReasonCode` | `FALLBACK_SUCCESS`, `RETRY_THRESHOLD`, `POOR_SIGNAL`, `CONGESTION`, `LOW_TTL`, `LOW_SUCCESS_RATE` | -| `SecurityWarningCode` | `SENDER_ADDRESS_MISMATCH`, `TRANSPORT_IDENTITY_MISMATCH`, `CONTROL_SIGNATURE_INVALID`, `UNSIGNED_CONTROL_REJECTED`, `MEDIA_SENDER_GROUP_MISMATCH`, `PLAINTEXT_SEND`, `PLAINTEXT_RECEIVE_REJECTED`, `SESSION_SENDER_GROUP_MISMATCH`, `SESSION_REKEY_TRIGGERED`, `NOSTR_KEY_PACKAGE_SLOT_EXHAUSTED`, `PUSH_KEY_PACKAGE_POOL_EXHAUSTED`, `RELAY_ADDRESS_BINDING_MISMATCH`, `RELAY_ADDRESS_DECLARATION_REFUSED`, `GROUP_LEAF_IDENTITY_UNPROVEN`, `STALE_CONTROL_FRAME`, `GATEWAY_ADDRESS_BINDING_MISMATCH`, `GATEWAY_ADDRESS_DECLARATION_REFUSED` | +| `SecurityWarningCode` | `SENDER_ADDRESS_MISMATCH`, `TRANSPORT_IDENTITY_MISMATCH`, `CONTROL_SIGNATURE_INVALID`, `UNSIGNED_CONTROL_REJECTED`, `MEDIA_SENDER_GROUP_MISMATCH`, `PLAINTEXT_SEND`, `PLAINTEXT_RECEIVE_REJECTED`, `SESSION_SENDER_GROUP_MISMATCH`, `SESSION_REKEY_TRIGGERED`, `NOSTR_KEY_PACKAGE_SLOT_EXHAUSTED`, `PUSH_KEY_PACKAGE_POOL_EXHAUSTED`, `RELAY_ADDRESS_BINDING_MISMATCH`, `RELAY_ADDRESS_DECLARATION_REFUSED`, `GROUP_LEAF_IDENTITY_UNPROVEN`, `STALE_CONTROL_FRAME`, `GATEWAY_ADDRESS_BINDING_MISMATCH`, `GATEWAY_ADDRESS_DECLARATION_REFUSED`, `KEY_PACKAGE_OUTSIDE_VALIDITY_WINDOW` | The transport name is spelled two ways by the engine, and a client accepts both. `message_received`, `message_delivered`, the DORS events and a From 320df7ee07998232b66cf8383c2264b234baa923 Mon Sep 17 00:00:00 2001 From: bahdotsh Date: Tue, 6 Oct 2026 10:21:30 +0530 Subject: [PATCH 2/2] fix(protocol,mls): report a clock-refused key package on every route The previous commit reported a key package refused by this device's clock, but only from the arrival-time route. The send path has its own near-identical copy of import-then-create-session, and that copy still did a bare `?` on the import. It is the route taken on the first send after a restart (the warning throttle lives in memory and the package comes back from storage) and whenever auto key exchange is off. There the send kick swallowed the error into a log line and queued the message "anyway". The app heard nothing. Which is exactly the failure this whole change exists to fix, just on the other half of the pair. It also treated both ends of the window as a clock fault. A window that has not started is one. A window that has closed may be one, or may just be an old package: the receiver anchors its cached expiry to when the frame arrived, using a remaining lifetime the sender computed when it built the frame. So a relay that held the package for days hands us a package whose cached expiry overshoots its real not_after. In that gap we told the user to check a clock that was fine, and then kept the package, so every attempt failed hard until the cached expiry. Let's fix both. One engine helper now admits a pending package for both routes and does the reporting, and the throttle keeps it to one event per peer whichever route fires first. The MLS error says which end refused the package. A window that has not started keeps the package, since it becomes valid once the clocks agree. A closed one is discarded the same way an expired package already is, and the reason names both causes. OpenMLS exposes no lifetime on an unvalidated KeyPackageIn outside its test-utils feature, so the direction is read back through its serde form, and only on the failure path. That is not pretty, but the alternative is walking the TLS encoding by hand, which is a second key package codec. Please don't do that. If the shape ever changes, the answer falls back to "not started", which is how the previous commit behaved, and the tests on both ends will say so. --- CHANGELOG.md | 11 +- bindings/react-native/src/types.ts | 9 +- crates/offline-protocol-mls/src/error.rs | 23 ++- crates/offline-protocol-mls/src/manager.rs | 95 ++++++++-- crates/offline-protocol/src/events.rs | 14 +- crates/offline-protocol/src/protocol/mod.rs | 91 +++++++--- crates/offline-protocol/src/protocol/send.rs | 11 +- .../src/protocol/tests/mod.rs | 162 +++++++++++++++++- docs/security/threat-model.md | 7 +- 9 files changed, 357 insertions(+), 66 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d15a02f5..96a63cc71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -354,9 +354,14 @@ archived by series under [docs/changelog/](docs/changelog/); see the online (one set to 2024, the other to 2025) reproduced it. The refusal now raises a `security_warning` with the new code `KEY_PACKAGE_OUTSIDE_VALIDITY_WINDOW`, once per peer, and is logged at - `warn`; `MlsError::KeyPackageOutsideValidityWindow` replaces the - `InvalidKeyPackage` text for this one refusal on every route that admits a - peer's package. The window itself is unchanged. TypeScript's + `warn`, whether the package was refused on arrival or on the first send + after a restart. `MlsError::KeyPackageOutsideValidityWindow { expired }` + replaces the `InvalidKeyPackage` text for this one refusal on every route + that admits a peer's package. A package whose window has already closed is + discarded like any expired package, because it may be an old package a relay + held rather than a clock, and kept it failed every attempt until its cached + expiry; one whose window has not started is kept. The window itself is + unchanged. TypeScript's `SecurityWarningCode` union gains the code, so an exhaustive `switch` over it needs a new arm. - **An old message no longer comes back as a push notification, again and diff --git a/bindings/react-native/src/types.ts b/bindings/react-native/src/types.ts index 2f7c3b5a6..f4a728cf7 100644 --- a/bindings/react-native/src/types.ts +++ b/bindings/react-native/src/types.ts @@ -2608,9 +2608,12 @@ export type SecurityWarningCode = * no encrypted session forms: messages and connection requests wait until * the clocks agree. This is what happens between phones that have not been * online to set their time. Many `peer_id`s is this device's clock; one is that - * peer's. Reported once per peer, and `peer_id` is a claim, since the package - * has not proved its sender when its window is checked. Show the user a - * prompt to check the date and time. + * peer's. A window that has already closed may instead be an old package a + * relay held for days; that package is discarded and the next one the peer + * sends replaces it, and `reason` names both causes. Reported once per peer, + * whether the refusal came when the package arrived or on a later send, and + * `peer_id` is a claim, since the package has not proved its sender when its + * window is checked. Show the user a prompt to check the date and time. */ export interface SecurityWarningEvent extends BaseEvent { type: 'security_warning'; diff --git a/crates/offline-protocol-mls/src/error.rs b/crates/offline-protocol-mls/src/error.rs index 829301662..2720a90b1 100644 --- a/crates/offline-protocol-mls/src/error.rs +++ b/crates/offline-protocol-mls/src/error.rs @@ -87,8 +87,25 @@ pub enum MlsError { /// `KEY_PACKAGE_OUTSIDE_VALIDITY_WINDOW`; before that it was a debug line, /// and two phones a year apart sat on a pending connection request with /// nothing anywhere to say why. - #[error("Key package is outside its validity window by this device's clock")] - KeyPackageOutsideValidityWindow, + /// + /// `expired` says which end refused it, because the two have different + /// remedies. A window that has not started (`false`) is a clock, this + /// device's or the peer's, and the package becomes valid once the clocks + /// agree. A window that has closed (`true`) is this device's clock running + /// ahead *or* a package that is simply old: a relay can hold a package for + /// days, and the receiver's cached expiry is anchored to when the frame + /// arrived, not to when the package was minted. Blaming the clock for the + /// second, and keeping the package, would fail every attempt with a + /// correct clock until the cached expiry. + #[error( + "Key package is outside its validity window by this device's clock: {}", + if *.expired { "it has already closed" } else { "it has not started yet" } + )] + KeyPackageOutsideValidityWindow { + /// `true` when the window closed before this device's `now`, `false` + /// when it has not opened yet. + expired: bool, + }, /// Failed to create a group. #[error("Group creation failed: {0}")] @@ -524,7 +541,7 @@ impl MlsError { Self::CredentialCreation(_) => "MLS credential creation failed", Self::KeyPackageCreation(_) => "Key package creation failed", Self::InvalidKeyPackage(_) => "The peer's key package was malformed or unusable", - Self::KeyPackageOutsideValidityWindow => { + Self::KeyPackageOutsideValidityWindow { .. } => { "The peer's key package is not valid at this device's time" } Self::GroupCreation(_) => "MLS group creation failed", diff --git a/crates/offline-protocol-mls/src/manager.rs b/crates/offline-protocol-mls/src/manager.rs index b2bb87f7e..a31b1454f 100644 --- a/crates/offline-protocol-mls/src/manager.rs +++ b/crates/offline-protocol-mls/src/manager.rs @@ -673,9 +673,7 @@ impl MlsManager { offline_protocol_core::validate_id_chars(user_id, "User ID") .map_err(|e| MlsError::InvalidUserId(e.to_string()))?; - let key_package_in = KeyPackageIn::tls_deserialize_exact(key_package_data) - .map_err(|e| MlsError::InvalidKeyPackage(e.to_string()))?; - let key_package = self.validate_peer_key_package(key_package_in)?; + let key_package = self.validate_peer_key_package(key_package_data)?; Self::verify_credential_identity(&key_package, user_id)?; Self::verify_address_binding(&key_package, user_id)?; @@ -696,11 +694,8 @@ impl MlsManager { .load(key_type, user_id)? .ok_or_else(|| MlsError::NoKeyPackage(user_id.to_string()))?; - let key_package_in = KeyPackageIn::tls_deserialize_exact(&data) - .map_err(|e| MlsError::InvalidKeyPackage(e.to_string()))?; - // Validate using the crypto backend - let key_package = self.validate_peer_key_package(key_package_in)?; + let key_package = self.validate_peer_key_package(&data)?; // Defense in depth: entries written to storage out-of-band must not // come back attributed to the wrong user. Both checks run again here @@ -720,17 +715,78 @@ impl MlsManager { /// package refused for its validity window comes back as /// [`MlsError::KeyPackageOutsideValidityWindow`] on every route that /// admits one (import, the cache read, adding a group member) rather than - /// as free text on some of them. The window is the only refusal whose - /// usual cause is a clock, and the caller reports it as such. - fn validate_peer_key_package(&self, key_package_in: KeyPackageIn) -> Result { - key_package_in + /// as free text on some of them, so the engine can report it. The window + /// is the only refusal whose usual cause is a clock. + /// + /// Takes the bytes rather than a parsed package because `validate` + /// consumes the package, and which end of the window refused it is read + /// back from the bytes only on that failure, so the accepting path pays + /// nothing for it. + fn validate_peer_key_package(&self, key_package_data: &[u8]) -> Result { + KeyPackageIn::tls_deserialize_exact(key_package_data) + .map_err(|e| MlsError::InvalidKeyPackage(e.to_string()))? .validate(self.provider.crypto(), ProtocolVersion::Mls10) .map_err(|e| match e { - KeyPackageVerifyError::InvalidLifetime => MlsError::KeyPackageOutsideValidityWindow, + KeyPackageVerifyError::InvalidLifetime => { + MlsError::KeyPackageOutsideValidityWindow { + expired: Self::window_has_closed(key_package_data), + } + } other => MlsError::InvalidKeyPackage(other.to_string()), }) } + /// Whether a package OpenMLS refused for its lifetime was refused because + /// the window has already closed, rather than because it has not opened. + /// + /// OpenMLS exposes no lifetime on an unvalidated `KeyPackageIn` outside + /// its `test-utils` feature, so the window is read from its serde form: the + /// leaf node's `Lifetime` is the only `{not_before, not_after}` object in a + /// key package. Read through OpenMLS's own types rather than by walking the + /// TLS encoding by hand, which would be a second key package codec. If the + /// shape ever changes the answer is `false`, the refusal is reported as a + /// clock and the package is kept, which is how this refusal was handled + /// before the two ends were told apart; the tests on both ends pin it. + /// + /// OpenMLS admits `not_before < now < not_after`, so a refused package + /// whose `not_after` is at or before `now` closed, and any other did not + /// start. A clock before the epoch reads as "not started", which is right: + /// that clock is behind. + fn window_has_closed(key_package_data: &[u8]) -> bool { + fn find_lifetime(value: &serde_json::Value) -> Option { + match value { + serde_json::Value::Object(map) => { + if map + .get("not_before") + .and_then(serde_json::Value::as_u64) + .is_some() + { + if let Some(not_after) = + map.get("not_after").and_then(serde_json::Value::as_u64) + { + return Some(not_after); + } + } + map.values().find_map(find_lifetime) + } + serde_json::Value::Array(items) => items.iter().find_map(find_lifetime), + _ => None, + } + } + + let Some(not_after) = KeyPackageIn::tls_deserialize_exact(key_package_data) + .ok() + .and_then(|package| serde_json::to_value(&package).ok()) + .and_then(|value| find_lifetime(&value)) + else { + return false; + }; + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|now| now.as_secs() >= not_after) + .unwrap_or(false) + } + /// Requires a key package's validity window to be no wider than this /// install admits. /// @@ -1117,10 +1173,7 @@ impl MlsManager { invitee_user_id: &str, member_key_package: &[u8], ) -> Result<(WelcomeMessage, EncryptedMessage)> { - let key_package = self.validate_peer_key_package( - KeyPackageIn::tls_deserialize_exact(member_key_package) - .map_err(|e| MlsError::InvalidKeyPackage(e.to_string()))?, - )?; + let key_package = self.validate_peer_key_package(member_key_package)?; Self::verify_credential_identity(&key_package, invitee_user_id)?; Self::verify_address_binding(&key_package, invitee_user_id)?; @@ -4954,7 +5007,10 @@ mod tests { .import_key_package(&peer, &bytes) .expect_err("a window that starts in a year is not valid now"); assert!( - matches!(err, MlsError::KeyPackageOutsideValidityWindow), + matches!( + err, + MlsError::KeyPackageOutsideValidityWindow { expired: false } + ), "refused for the wrong reason: {err:?}" ); } @@ -4974,7 +5030,10 @@ mod tests { .import_key_package(&peer, &bytes) .expect_err("a window that closed a month ago is not valid now"); assert!( - matches!(err, MlsError::KeyPackageOutsideValidityWindow), + matches!( + err, + MlsError::KeyPackageOutsideValidityWindow { expired: true } + ), "refused for the wrong reason: {err:?}" ); } diff --git a/crates/offline-protocol/src/events.rs b/crates/offline-protocol/src/events.rs index b08b700c1..db72b9cc5 100644 --- a/crates/offline-protocol/src/events.rs +++ b/crates/offline-protocol/src/events.rs @@ -346,9 +346,17 @@ pub enum SecurityWarningCode { /// been online are where this happens. Many different peers is this /// device's clock; one peer while others are fine is that peer's. /// - /// The package has not proved its sender when the window is checked, so - /// the peer named is a claim, and the event is reported at most once per - /// peer. + /// A window that has already closed is not always a clock: a relay can + /// hold a package for days, and the receiver anchors its cached expiry to + /// when the frame arrived. That package is discarded, as an expired one + /// is, and the `reason` says either cause; the next package the peer sends + /// replaces it. A window that has not started is kept, since it becomes + /// valid once the clocks agree. + /// + /// Raised on every route that admits a pending package: when it arrives, + /// and on the first send after a restart. The package has not proved its + /// sender when the window is checked, so the peer named is a claim, and + /// the event is reported at most once per peer. KeyPackageOutsideValidityWindow, } diff --git a/crates/offline-protocol/src/protocol/mod.rs b/crates/offline-protocol/src/protocol/mod.rs index c1c49a698..41d276798 100644 --- a/crates/offline-protocol/src/protocol/mod.rs +++ b/crates/offline-protocol/src/protocol/mod.rs @@ -2971,33 +2971,7 @@ impl OfflineProtocol { self.pending_key_packages.remove(peer_id); self.delete_peer_key_package_from_storage(peer_id); } else { - let imported = { - let manager = mls - .read() - .map_err(|_| Error::Other("MLS lock poisoned".to_string()))?; - manager.import_key_package(peer_id, &received_pkg.key_package_data) - }; - if let Err(offline_protocol_mls::MlsError::KeyPackageOutsideValidityWindow) = - &imported - { - // The usual cause is a clock, on one side or the other, and - // it blocks every session with this peer for as long as the - // clocks disagree. Logged and reported, because this was a - // debug line and nothing else. - warn!( - peer_id = %peer_id, - "Peer key package is outside its validity window by this \ - device's clock; this device cannot start a session with the \ - peer until the clocks agree" - ); - self.warn_control_gate_rejection( - peer_id, - crate::events::SecurityWarningCode::KeyPackageOutsideValidityWindow, - "The peer's key package is not valid at this device's time. \ - Check this device's date and time, then the peer's.", - ); - } - imported?; + self.import_pending_key_package(&mls, peer_id, &received_pkg.key_package_data)?; // Create session and get welcome message let welcome = { @@ -3057,6 +3031,69 @@ impl OfflineProtocol { Err(Error::SessionNotReady(self.establishment_state(peer_id)?)) } + /// Imports `peer_id`'s pending key package into MLS, reporting a refusal + /// by this device's clock. + /// + /// The one place the engine admits a pending package, so that the refusal + /// is reported whichever route reached it: `establish_secure_session`, + /// taken when a package arrives, and the send path's + /// `ensure_session_establishment`, taken on the first send after a + /// restart or with `auto_key_exchange` off. Reported on only the first, + /// the restart case left a message queued "anyway" with nothing to say why, + /// which is the failure the code exists for. The throttle makes a second + /// route free: one event per peer either way. + /// + /// A window that has not started keeps the package, since it becomes valid + /// once the clocks agree. A window that has closed discards it, exactly as + /// an expired cached package is discarded, because it may be a package a + /// relay held for days rather than a clock, and kept it would fail every + /// attempt until its cached expiry. The next package the peer sends + /// replaces it. + pub(super) fn import_pending_key_package( + &mut self, + mls: &Arc>, + peer_id: &str, + key_package_data: &[u8], + ) -> Result<()> { + let imported = { + let manager = mls + .read() + .map_err(|_| Error::Other("MLS lock poisoned".to_string()))?; + manager.import_key_package(peer_id, key_package_data) + }; + if let Err(offline_protocol_mls::MlsError::KeyPackageOutsideValidityWindow { expired }) = + &imported + { + let reason = if *expired { + warn!( + peer_id = %peer_id, + "Peer key package's validity window has closed by this device's \ + clock; discarding it until the peer sends a fresh one" + ); + self.pending_key_packages.remove(peer_id); + self.delete_peer_key_package_from_storage(peer_id); + "The peer's key package has expired by this device's clock. Either \ + this device's date is ahead, or the package is older than its 30-day \ + window; it is discarded and the next one the peer sends replaces it." + } else { + warn!( + peer_id = %peer_id, + "Peer key package's validity window has not started by this \ + device's clock; this device cannot start a session with the peer \ + until the clocks agree" + ); + "The peer's key package is not valid yet at this device's time. \ + Check this device's date and time, then the peer's." + }; + self.warn_control_gate_rejection( + peer_id, + crate::events::SecurityWarningCode::KeyPackageOutsideValidityWindow, + reason, + ); + } + Ok(imported?) + } + /// Checks if a pending key package is available for a peer. /// /// This can be used to check if session establishment is possible diff --git a/crates/offline-protocol/src/protocol/send.rs b/crates/offline-protocol/src/protocol/send.rs index 0d9c69c7e..9ae73ef1d 100644 --- a/crates/offline-protocol/src/protocol/send.rs +++ b/crates/offline-protocol/src/protocol/send.rs @@ -898,12 +898,11 @@ impl OfflineProtocol { self.pending_key_packages.remove(recipient); self.delete_peer_key_package_from_storage(recipient); } else { - { - let manager = mls - .read() - .map_err(|_| Error::Other("MLS lock poisoned".to_string()))?; - manager.import_key_package(recipient, &received_pkg.key_package_data)?; - } + self.import_pending_key_package( + mls, + recipient, + &received_pkg.key_package_data, + )?; // Create session and send welcome message let welcome = { diff --git a/crates/offline-protocol/src/protocol/tests/mod.rs b/crates/offline-protocol/src/protocol/tests/mod.rs index b10cc49bf..3b8436be1 100644 --- a/crates/offline-protocol/src/protocol/tests/mod.rs +++ b/crates/offline-protocol/src/protocol/tests/mod.rs @@ -40265,7 +40265,11 @@ fn a_key_package_outside_its_window_by_our_clock_is_reported_once_per_peer() { assert!( matches!( err, - Error::Mls(offline_protocol_mls::MlsError::KeyPackageOutsideValidityWindow) + Error::Mls( + offline_protocol_mls::MlsError::KeyPackageOutsideValidityWindow { + expired: false + } + ) ), "refused for the wrong reason: {err:?}" ); @@ -40280,6 +40284,162 @@ fn a_key_package_outside_its_window_by_our_clock_is_reported_once_per_peer() { )], "exactly one warning naming the peer, whatever the retry count" ); + assert!( + alice.has_pending_key_package(&bob), + "a window that has not started becomes valid once the clocks agree, so \ + the package is kept" + ); +} + +/// Collects `(peer_id, code, reason)` for every security warning `protocol` +/// raises. +fn collect_security_warnings( + protocol: &mut OfflineProtocol, +) -> Arc>> { + let warnings = Arc::new(Mutex::new(Vec::new())); + let sink = Arc::clone(&warnings); + protocol.on_event(move |e| { + if let Event::SecurityWarning { + peer_id, + reason_code, + reason, + } = e + { + sink.lock().unwrap().push((peer_id, reason_code, reason)); + } + }); + warnings +} + +/// Places a key package for `peer` whose window runs from `not_before` to +/// `not_after` in `protocol`'s pending map, as a frame that arrived would. +fn insert_pending_key_package_with_window( + protocol: &mut OfflineProtocol, + peer: &str, + not_before: u64, + not_after: u64, +) { + let bytes = offline_protocol_mls::MlsManager::key_package_with_window_for_testing( + peer, not_before, not_after, + ) + .unwrap(); + let pkg = ReceivedKeyPackage { + key_package_data: bytes, + local_expires_at_ms: u64::MAX, + }; + protocol.persist_peer_key_package(peer, &pkg); + protocol.pending_key_packages.insert(peer.to_string(), pkg); +} + +/// The send path imports a pending package through its own route, and that +/// route is the one taken on the first send after a restart (the warning +/// throttle is in memory, the package is restored from storage) or whenever +/// `auto_key_exchange` is off. Reported only on the arrival route, the message +/// queued "anyway" and the app heard nothing: the failure the code exists for. +#[test] +fn a_send_reports_a_key_package_outside_its_window_once() { + let mut config = create_test_config_for_user("alice"); + config.encryption.auto_key_exchange = false; + // The stock default: a send to a peer with no session queues behind + // establishment and kicks it, rather than going out in the clear. + config.encryption.require_encryption = true; + config.encryption.store_pending = true; + let mut alice = OfflineProtocol::new(config).unwrap(); + alice + .initialize_mls_for_test(Arc::new(crate::mls::InMemoryStorage::new())) + .unwrap(); + alice.start().unwrap(); + let warnings = collect_security_warnings(&mut alice); + + let now = Utc::now().timestamp() as u64; + let year = 365 * 24 * 3600; + let bob = id("bob"); + insert_pending_key_package_with_window( + &mut alice, + &bob, + now + year - 3600, + now + year + 30 * 24 * 3600, + ); + + for _ in 0..3 { + alice + .send_message(&bob, "hello", None, None::) + .expect("the message queues behind establishment"); + } + + let seen: Vec<_> = warnings + .lock() + .unwrap() + .iter() + .map(|(peer, code, _)| (peer.clone(), *code)) + .collect(); + assert_eq!( + seen, + vec![( + bob.clone(), + SecurityWarningCode::KeyPackageOutsideValidityWindow + )], + "the send path reports the refusal, once however many sends retry it" + ); + assert!(alice.has_pending_key_package(&bob)); +} + +/// A window that has already closed is not necessarily a clock: a relay can +/// hold a package for days, and the cached expiry is anchored to when the +/// frame arrived. So it is reported without telling the user their clock is +/// wrong, and discarded like any expired package, rather than kept to fail +/// every attempt until the cached expiry. +#[test] +fn a_key_package_whose_window_closed_is_reported_and_discarded() { + let mut alice = protocol_with_mls("alice"); + let warnings = collect_security_warnings(&mut alice); + + let now = Utc::now().timestamp() as u64; + let day = 24 * 3600; + let bob = id("bob"); + insert_pending_key_package_with_window(&mut alice, &bob, now - 60 * day, now - 30 * day); + assert!( + alice.load_peer_key_package_from_storage(&bob).is_some(), + "negative control: the package starts out persisted" + ); + let err = alice + .establish_secure_session(&bob) + .expect_err("no session can form from a package whose window closed"); + assert!( + matches!( + err, + Error::Mls( + offline_protocol_mls::MlsError::KeyPackageOutsideValidityWindow { expired: true } + ) + ), + "refused for the wrong reason: {err:?}" + ); + + assert!( + !alice.has_pending_key_package(&bob), + "a closed window never reopens on a correct clock, so the package goes" + ); + assert!( + alice.load_peer_key_package_from_storage(&bob).is_none(), + "and its durable copy with it, or the next launch restores it" + ); + let seen = warnings.lock().unwrap().clone(); + assert_eq!(seen.len(), 1, "one warning: {seen:?}"); + let (peer, code, reason) = &seen[0]; + assert_eq!( + (peer, *code), + (&bob, SecurityWarningCode::KeyPackageOutsideValidityWindow) + ); + assert!( + reason.contains("older than its 30-day window"), + "the reason names the stale package, not only the clock: {reason}" + ); + + // Once gone, the next attempt is plain "not ready", not the same refusal. + assert!(matches!( + alice.establish_secure_session(&bob), + Err(Error::SessionNotReady(_)) + )); } /// The refusal is reported under its own code, so an integrator can tell a diff --git a/docs/security/threat-model.md b/docs/security/threat-model.md index 4eae0517f..abb46feb2 100644 --- a/docs/security/threat-model.md +++ b/docs/security/threat-model.md @@ -397,8 +397,11 @@ a new binary. A key package's validity window is judged the same way and fails the same way: it runs from an hour before minting to 30 days after, so a device more than an hour behind its peer cannot start a session (the peer still can), and past 30 days neither side can and no session forms. The refusal is -reported under `KEY_PACKAGE_OUTSIDE_VALIDITY_WINDOW`, once per peer, and the -window itself is not relaxed. A leaf node is required to have a time source at pairing for this +reported under `KEY_PACKAGE_OUTSIDE_VALIDITY_WINDOW`, once per peer, on every +route that admits a pending package, and the window itself is not relaxed. A +window that has already closed is not always a clock (a relay may have held the +package for days), so that package is discarded and the peer's next one +replaces it. A leaf node is required to have a time source at pairing for this among other reasons; see [leaf provisioning](../spec/leaf-provisioning.md).