Fix JMAP: Stale push subscription can block verification of a new one
This commit is contained in:
@@ -17,6 +17,7 @@ If you are upgrading from v0.16.x, replace the binary (or run `docker pull`). If
|
|||||||
- JMAP:
|
- JMAP:
|
||||||
- Read-only sharee cannot set `isSubscribed` on a shared mailbox.
|
- 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.
|
- 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.
|
- `PushSubscription/set` rejects the unpadded base64url keys the W3C Push API produces.
|
||||||
- `Email/import` does not send push notifications for imported messages.
|
- `Email/import` does not send push notifications for imported messages.
|
||||||
- `CalendarEvent/set` silently ignores `ifInState`.
|
- `CalendarEvent/set` silently ignores `ifInState`.
|
||||||
|
|||||||
@@ -317,6 +317,16 @@ fn validate_push_value(
|
|||||||
.and_then(|v| v.as_str())
|
.and_then(|v| v.as_str())
|
||||||
.and_then(|v| URL_SAFE_INDIFFERENT.decode(v.as_ref()).ok()),
|
.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 });
|
push.keys = Some(Keys { auth, p256dh });
|
||||||
} else {
|
} else {
|
||||||
return Err(SetError::invalid_properties()
|
return Err(SetError::invalid_properties()
|
||||||
|
|||||||
@@ -11,7 +11,7 @@ use common::{
|
|||||||
auth::BuildAccessToken,
|
auth::BuildAccessToken,
|
||||||
ipc::{PushEvent, PushNotification},
|
ipc::{PushEvent, PushNotification},
|
||||||
};
|
};
|
||||||
use email::push::PushSubscriptions;
|
use email::push::{PushSubscription, PushSubscriptions};
|
||||||
use std::{
|
use std::{
|
||||||
collections::hash_map::Entry,
|
collections::hash_map::Entry,
|
||||||
sync::Arc,
|
sync::Arc,
|
||||||
@@ -161,6 +161,7 @@ pub fn spawn_push_manager(inner: Arc<Inner>) -> mpsc::Sender<Event> {
|
|||||||
|
|
||||||
// Process subscriptions
|
// Process subscriptions
|
||||||
let current_time = now();
|
let current_time = now();
|
||||||
|
let mut newest_unverified: Option<Arc<PushSubscription>> = None;
|
||||||
for subscription in subscriptions
|
for subscription in subscriptions
|
||||||
.subscriptions
|
.subscriptions
|
||||||
.into_iter()
|
.into_iter()
|
||||||
@@ -194,54 +195,58 @@ pub fn spawn_push_manager(inner: Arc<Inner>) -> mpsc::Sender<Event> {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
let current_time = Instant::now();
|
match &newest_unverified {
|
||||||
|
Some(existing) if existing.id >= subscription.id => {}
|
||||||
#[cfg(feature = "test_mode")]
|
_ => newest_unverified = Some(subscription),
|
||||||
if subscription.url.contains("skip_checks") {
|
|
||||||
last_verify.insert(
|
|
||||||
account_id,
|
|
||||||
current_time
|
|
||||||
- (push_verify_timeout + Duration::from_millis(1)),
|
|
||||||
);
|
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if last_verify
|
if let Some(subscription) = newest_unverified {
|
||||||
.get(&account_id)
|
let current_time = Instant::now();
|
||||||
.map(|last_verify| {
|
|
||||||
current_time - *last_verify > push_verify_timeout
|
#[cfg(feature = "test_mode")]
|
||||||
})
|
if subscription.url.contains("skip_checks") {
|
||||||
.unwrap_or(true)
|
last_verify.insert(
|
||||||
{
|
account_id,
|
||||||
tokio::spawn(async move {
|
current_time - (push_verify_timeout + Duration::from_millis(1)),
|
||||||
http_request(
|
);
|
||||||
&subscription,
|
}
|
||||||
format!(
|
|
||||||
concat!(
|
if last_verify
|
||||||
"{{\"@type\":\"PushVerification\",",
|
.get(&account_id)
|
||||||
"\"pushSubscriptionId\":\"{}\",",
|
.map(|last_verify| {
|
||||||
"\"verificationCode\":\"{}\"}}"
|
current_time - *last_verify > push_verify_timeout
|
||||||
),
|
})
|
||||||
Id::from(subscription.id),
|
.unwrap_or(true)
|
||||||
subscription.verification_code
|
{
|
||||||
)
|
tokio::spawn(async move {
|
||||||
.into_bytes(),
|
http_request(
|
||||||
push_timeout,
|
&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);
|
last_verify.insert(account_id, current_time);
|
||||||
} else {
|
} else {
|
||||||
trc::event!(
|
trc::event!(
|
||||||
PushSubscription(PushSubscriptionEvent::Error),
|
PushSubscription(PushSubscriptionEvent::Error),
|
||||||
Details = "Failed to verify push subscription",
|
Details = "Failed to verify push subscription",
|
||||||
Url = subscription.url.clone(),
|
Url = subscription.url.clone(),
|
||||||
AccountId = account_id,
|
AccountId = account_id,
|
||||||
Reason = "Too many requests"
|
Reason = "Too many requests"
|
||||||
);
|
);
|
||||||
|
|
||||||
continue;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user