diff --git a/CHANGELOG.md b/CHANGELOG.md index e96053c4..14e0719b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ If you are upgrading from v0.16.x, replace the binary (or run `docker pull`). If - JMAP: - Read-only sharee cannot set `isSubscribed` on a shared mailbox. - Web Push payloads with `Content-Encoding: aes128gcm` should not be base64-encoded but sent as raw bytes. + - Stale push subscription can block verification of a new one. - `PushSubscription/set` rejects the unpadded base64url keys the W3C Push API produces. - `Email/import` does not send push notifications for imported messages. - `CalendarEvent/set` silently ignores `ifInState`. diff --git a/crates/jmap/src/push/set.rs b/crates/jmap/src/push/set.rs index 847a646c..27aab7cb 100644 --- a/crates/jmap/src/push/set.rs +++ b/crates/jmap/src/push/set.rs @@ -317,6 +317,16 @@ fn validate_push_value( .and_then(|v| v.as_str()) .and_then(|v| URL_SAFE_INDIFFERENT.decode(v.as_ref()).ok()), ) { + if p256::PublicKey::from_sec1_bytes(&p256dh).is_err() { + return Err(SetError::invalid_properties() + .with_property(property.clone()) + .with_description("Invalid P-256 ECDH public key.")); + } + if auth.len() != 16 { + return Err(SetError::invalid_properties() + .with_property(property.clone()) + .with_description("Invalid auth secret, expected 16 octets.")); + } push.keys = Some(Keys { auth, p256dh }); } else { return Err(SetError::invalid_properties() diff --git a/crates/services/src/state_manager/push.rs b/crates/services/src/state_manager/push.rs index 47587476..9cdbf87d 100644 --- a/crates/services/src/state_manager/push.rs +++ b/crates/services/src/state_manager/push.rs @@ -11,7 +11,7 @@ use common::{ auth::BuildAccessToken, ipc::{PushEvent, PushNotification}, }; -use email::push::PushSubscriptions; +use email::push::{PushSubscription, PushSubscriptions}; use std::{ collections::hash_map::Entry, sync::Arc, @@ -161,6 +161,7 @@ pub fn spawn_push_manager(inner: Arc) -> mpsc::Sender { // Process subscriptions let current_time = now(); + let mut newest_unverified: Option> = None; for subscription in subscriptions .subscriptions .into_iter() @@ -194,54 +195,58 @@ pub fn spawn_push_manager(inner: Arc) -> mpsc::Sender { } } } else { - let current_time = Instant::now(); - - #[cfg(feature = "test_mode")] - if subscription.url.contains("skip_checks") { - last_verify.insert( - account_id, - current_time - - (push_verify_timeout + Duration::from_millis(1)), - ); + match &newest_unverified { + Some(existing) if existing.id >= subscription.id => {} + _ => newest_unverified = Some(subscription), } + } + } - if last_verify - .get(&account_id) - .map(|last_verify| { - current_time - *last_verify > push_verify_timeout - }) - .unwrap_or(true) - { - tokio::spawn(async move { - http_request( - &subscription, - format!( - concat!( - "{{\"@type\":\"PushVerification\",", - "\"pushSubscriptionId\":\"{}\",", - "\"verificationCode\":\"{}\"}}" - ), - Id::from(subscription.id), - subscription.verification_code - ) - .into_bytes(), - push_timeout, + if let Some(subscription) = newest_unverified { + let current_time = Instant::now(); + + #[cfg(feature = "test_mode")] + if subscription.url.contains("skip_checks") { + last_verify.insert( + account_id, + current_time - (push_verify_timeout + Duration::from_millis(1)), + ); + } + + if last_verify + .get(&account_id) + .map(|last_verify| { + current_time - *last_verify > push_verify_timeout + }) + .unwrap_or(true) + { + tokio::spawn(async move { + http_request( + &subscription, + format!( + concat!( + "{{\"@type\":\"PushVerification\",", + "\"pushSubscriptionId\":\"{}\",", + "\"verificationCode\":\"{}\"}}" + ), + Id::from(subscription.id), + subscription.verification_code ) - .await; - }); + .into_bytes(), + push_timeout, + ) + .await; + }); - last_verify.insert(account_id, current_time); - } else { - trc::event!( - PushSubscription(PushSubscriptionEvent::Error), - Details = "Failed to verify push subscription", - Url = subscription.url.clone(), - AccountId = account_id, - Reason = "Too many requests" - ); - - continue; - } + last_verify.insert(account_id, current_time); + } else { + trc::event!( + PushSubscription(PushSubscriptionEvent::Error), + Details = "Failed to verify push subscription", + Url = subscription.url.clone(), + AccountId = account_id, + Reason = "Too many requests" + ); } }