fix(push): close E2EE-flip race by re-checking room encryption at reply send time, pre-flight credentials before optimistic echo, share callId-dedup helpers between FCM and Worker
This commit is contained in:
parent
06778702b2
commit
de348eb4fc
3 changed files with 79 additions and 22 deletions
|
|
@ -12,6 +12,7 @@ import androidx.core.app.NotificationCompat;
|
||||||
import androidx.core.app.RemoteInput;
|
import androidx.core.app.RemoteInput;
|
||||||
|
|
||||||
import org.json.JSONObject;
|
import org.json.JSONObject;
|
||||||
|
import org.json.JSONException;
|
||||||
|
|
||||||
import java.io.IOException;
|
import java.io.IOException;
|
||||||
import java.io.OutputStream;
|
import java.io.OutputStream;
|
||||||
|
|
@ -88,12 +89,11 @@ public class ReplyReceiver extends BroadcastReceiver {
|
||||||
|
|
||||||
final Context appContext = context.getApplicationContext();
|
final Context appContext = context.getApplicationContext();
|
||||||
|
|
||||||
// Optimistic local echo — appends a self-Person message to the
|
// Pre-flight validation BEFORE the optimistic echo. Posting a self
|
||||||
// conversation and re-posts, so the user sees their reply in the
|
// bubble first and then immediately stacking an error notif on top
|
||||||
// shade before the HTTP completes.
|
// is jarring UX; for predictable failures (logged out, freshly
|
||||||
long now = System.currentTimeMillis();
|
// encrypted room) we'd rather skip the echo and only surface the
|
||||||
VojoFirebaseMessagingService.appendOutgoingMessage(appContext, roomId, text, now);
|
// error.
|
||||||
|
|
||||||
final SharedPreferences prefs = appContext.getSharedPreferences(
|
final SharedPreferences prefs = appContext.getSharedPreferences(
|
||||||
VojoPollWorker.PREFS, Context.MODE_PRIVATE);
|
VojoPollWorker.PREFS, Context.MODE_PRIVATE);
|
||||||
final String token = prefs.getString(VojoPollWorker.KEY_ACCESS_TOKEN, null);
|
final String token = prefs.getString(VojoPollWorker.KEY_ACCESS_TOKEN, null);
|
||||||
|
|
@ -104,6 +104,28 @@ public class ReplyReceiver extends BroadcastReceiver {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Race guard for E2EE flip: the per-room metadata snapshot is
|
||||||
|
// refreshed by JS on m.room.encryption Timeline events, but a push
|
||||||
|
// delivered in the narrow window between the encryption state
|
||||||
|
// landing and the dump completing could still expose the reply
|
||||||
|
// action on a freshly-encrypted room. Re-read the snapshot
|
||||||
|
// synchronously here — Synapse does NOT enforce "no cleartext in
|
||||||
|
// encrypted rooms" at the spec level, so without this guard we'd
|
||||||
|
// leak the user's reply into an E2EE timeline as plaintext.
|
||||||
|
if (isRoomEncryptedAtSendTime(prefs, roomId)) {
|
||||||
|
Log.w(TAG, "onReceive: room flipped to encrypted between render and send, abort");
|
||||||
|
postReplyError(appContext, roomId);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Optimistic local echo — appends a self-Person message to the
|
||||||
|
// conversation and re-posts, so the user sees their reply in the
|
||||||
|
// shade before the HTTP completes. Only happens after pre-flight
|
||||||
|
// checks pass so the user doesn't see an echo for a reply we know
|
||||||
|
// will fail.
|
||||||
|
long now = System.currentTimeMillis();
|
||||||
|
VojoFirebaseMessagingService.appendOutgoingMessage(appContext, roomId, text, now);
|
||||||
|
|
||||||
final PendingResult pendingResult = goAsync();
|
final PendingResult pendingResult = goAsync();
|
||||||
final String txnId = "vojo-reply-" + UUID.randomUUID();
|
final String txnId = "vojo-reply-" + UUID.randomUUID();
|
||||||
EXECUTOR.execute(() -> {
|
EXECUTOR.execute(() -> {
|
||||||
|
|
@ -180,7 +202,7 @@ public class ReplyReceiver extends BroadcastReceiver {
|
||||||
ctx.getSystemService(Context.NOTIFICATION_SERVICE);
|
ctx.getSystemService(Context.NOTIFICATION_SERVICE);
|
||||||
if (nm == null) return;
|
if (nm == null) return;
|
||||||
try {
|
try {
|
||||||
String channel = VojoFirebaseMessagingService.CHANNEL_ID_DM_PUBLIC;
|
String channel = VojoFirebaseMessagingService.CHANNEL_ID_DM;
|
||||||
NotificationCompat.Builder b = new NotificationCompat.Builder(ctx, channel)
|
NotificationCompat.Builder b = new NotificationCompat.Builder(ctx, channel)
|
||||||
.setSmallIcon(R.mipmap.ic_launcher)
|
.setSmallIcon(R.mipmap.ic_launcher)
|
||||||
.setContentTitle(PushStrings.replyFailed(ctx))
|
.setContentTitle(PushStrings.replyFailed(ctx))
|
||||||
|
|
@ -197,4 +219,30 @@ public class ReplyReceiver extends BroadcastReceiver {
|
||||||
private static String trimTrailingSlash(String s) {
|
private static String trimTrailingSlash(String s) {
|
||||||
return (s != null && s.endsWith("/")) ? s.substring(0, s.length() - 1) : s;
|
return (s != null && s.endsWith("/")) ? s.substring(0, s.length() - 1) : s;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Synchronous re-check of the room's encryption flag at send time.
|
||||||
|
* Mirrors VojoFirebaseMessagingService.loadRoomMetadata's tolerant
|
||||||
|
* parse: legacy string-shape entries and missing flags both default
|
||||||
|
* to encrypted=true (privacy-first — refusing a reply on a falsely-
|
||||||
|
* flagged room is harmless; sending cleartext into a truly encrypted
|
||||||
|
* room is a privacy leak).
|
||||||
|
*/
|
||||||
|
private static boolean isRoomEncryptedAtSendTime(SharedPreferences prefs, String roomId) {
|
||||||
|
String raw = prefs.getString(VojoPollWorker.KEY_ROOM_NAMES, null);
|
||||||
|
if (raw == null || raw.isEmpty()) return true;
|
||||||
|
try {
|
||||||
|
JSONObject map = new JSONObject(raw);
|
||||||
|
if (!map.has(roomId) || map.isNull(roomId)) return true;
|
||||||
|
JSONObject obj = map.optJSONObject(roomId);
|
||||||
|
if (obj == null) {
|
||||||
|
// Legacy string-shape predates the encryption flag —
|
||||||
|
// assume encrypted to err on the side of privacy.
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
return obj.optBoolean("isEncrypted", true);
|
||||||
|
} catch (JSONException je) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -66,12 +66,11 @@ public class VojoFirebaseMessagingService extends MessagingService {
|
||||||
// after creation on API 26+, so any future config change (sound, vibration
|
// after creation on API 26+, so any future config change (sound, vibration
|
||||||
// pattern) bumps the version.
|
// pattern) bumps the version.
|
||||||
private static final String MESSAGE_CHANNEL_GROUP_ID = "vojo_messages_v1";
|
private static final String MESSAGE_CHANNEL_GROUP_ID = "vojo_messages_v1";
|
||||||
private static final String CHANNEL_ID_DM = "vojo_messages_dm_v1";
|
// Package-visible: ReplyReceiver reuses CHANNEL_ID_DM for its "reply
|
||||||
|
// failed" one-shot notification. Was previously a separate _PUBLIC
|
||||||
|
// alias; collapsed to a single package-private constant.
|
||||||
|
static final String CHANNEL_ID_DM = "vojo_messages_dm_v1";
|
||||||
private static final String CHANNEL_ID_GROUP = "vojo_messages_group_v1";
|
private static final String CHANNEL_ID_GROUP = "vojo_messages_group_v1";
|
||||||
// Package-visible mirror of CHANNEL_ID_DM so ReplyReceiver can post its
|
|
||||||
// one-shot "reply failed" error notification on the same channel without
|
|
||||||
// duplicating the literal.
|
|
||||||
static final String CHANNEL_ID_DM_PUBLIC = CHANNEL_ID_DM;
|
|
||||||
|
|
||||||
private static final String GROUP_KEY = "vojo_messages";
|
private static final String GROUP_KEY = "vojo_messages";
|
||||||
// NotificationChannel settings (vibration pattern, sound, importance) are
|
// NotificationChannel settings (vibration pattern, sound, importance) are
|
||||||
|
|
@ -105,8 +104,12 @@ public class VojoFirebaseMessagingService extends MessagingService {
|
||||||
* m.rtc.notification events that share the parent — re-ring for
|
* m.rtc.notification events that share the parent — re-ring for
|
||||||
* participant rejoin, FCM retry-with-fresh-event-id — see the mark and
|
* participant rejoin, FCM retry-with-fresh-event-id — see the mark and
|
||||||
* silently skip the post.
|
* silently skip the post.
|
||||||
|
*
|
||||||
|
* Package-private so VojoPollWorker can build the same key shape from
|
||||||
|
* its flatten path without duplicating the literal — keeps the FCM and
|
||||||
|
* polling paths trivially in sync.
|
||||||
*/
|
*/
|
||||||
private static String compositeCallDedupKey(String roomId, String callSessionId) {
|
static String compositeCallDedupKey(String roomId, String callSessionId) {
|
||||||
return "call:" + roomId + ":" + callSessionId;
|
return "call:" + roomId + ":" + callSessionId;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -121,8 +124,12 @@ public class VojoFirebaseMessagingService extends MessagingService {
|
||||||
* deployment config. We probe both shapes — and `content_call_id` as
|
* deployment config. We probe both shapes — and `content_call_id` as
|
||||||
* a third candidate for legacy MSC2746 calls — so the dedup works
|
* a third candidate for legacy MSC2746 calls — so the dedup works
|
||||||
* regardless of which encoding the homeserver's Sygnal happens to use.
|
* regardless of which encoding the homeserver's Sygnal happens to use.
|
||||||
|
*
|
||||||
|
* Package-private: VojoPollWorker's flatten path writes the
|
||||||
|
* `content_m.relates_to_event_id` form, so this same helper resolves
|
||||||
|
* both FCM and polling payloads.
|
||||||
*/
|
*/
|
||||||
private static String extractCallSessionId(Map<String, String> data) {
|
static String extractCallSessionId(Map<String, String> data) {
|
||||||
String[] candidates = new String[] {
|
String[] candidates = new String[] {
|
||||||
"content_m.relates_to_event_id",
|
"content_m.relates_to_event_id",
|
||||||
"content_m_relates_to_event_id",
|
"content_m_relates_to_event_id",
|
||||||
|
|
|
||||||
|
|
@ -291,13 +291,16 @@ public class VojoPollWorker extends Worker {
|
||||||
// missed-call. Without this, a session with one
|
// missed-call. Without this, a session with one
|
||||||
// FCM live-alert ring + one re-ring through
|
// FCM live-alert ring + one re-ring through
|
||||||
// polling would surface as both a CallStyle and
|
// polling would surface as both a CallStyle and
|
||||||
// a missed-call card.
|
// a missed-call card. Helpers live in
|
||||||
|
// VojoFirebaseMessagingService so the key shape
|
||||||
|
// stays in lock-step across FCM and polling.
|
||||||
String roomIdField = flattened.get("room_id");
|
String roomIdField = flattened.get("room_id");
|
||||||
String sessionId = flattened.get("content_m.relates_to_event_id");
|
String sessionId = VojoFirebaseMessagingService
|
||||||
if (sessionId == null) sessionId = flattened.get("content_call_id");
|
.extractCallSessionId(flattened);
|
||||||
|
String composite = null;
|
||||||
if (roomIdField != null && sessionId != null) {
|
if (roomIdField != null && sessionId != null) {
|
||||||
String composite =
|
composite = VojoFirebaseMessagingService
|
||||||
"call:" + roomIdField + ":" + sessionId;
|
.compositeCallDedupKey(roomIdField, sessionId);
|
||||||
if (NotificationDedup.wasNotified(ctx, composite)) {
|
if (NotificationDedup.wasNotified(ctx, composite)) {
|
||||||
if (ts > highestTsSeen) highestTsSeen = ts;
|
if (ts > highestTsSeen) highestTsSeen = ts;
|
||||||
treatAsNotRenderable = true;
|
treatAsNotRenderable = true;
|
||||||
|
|
@ -311,12 +314,11 @@ public class VojoPollWorker extends Worker {
|
||||||
// CallStyle.
|
// CallStyle.
|
||||||
posted = VojoFirebaseMessagingService
|
posted = VojoFirebaseMessagingService
|
||||||
.renderMissedCallNotification(ctx, flattened);
|
.renderMissedCallNotification(ctx, flattened);
|
||||||
if (posted && roomIdField != null && sessionId != null) {
|
if (posted && composite != null) {
|
||||||
// Mark the composite so the next polling
|
// Mark the composite so the next polling
|
||||||
// cycle observing a re-ring for the same
|
// cycle observing a re-ring for the same
|
||||||
// session doesn't double-post.
|
// session doesn't double-post.
|
||||||
NotificationDedup.markNotified(ctx,
|
NotificationDedup.markNotified(ctx, composite);
|
||||||
"call:" + roomIdField + ":" + sessionId);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} else if (isRtcType) {
|
} else if (isRtcType) {
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue