Decrypt E2EE notifications by gnprice · Pull Request #2194 · zulip/zulip-flutter

gnprice · 2026-03-03T05:56:25Z

Toward #1764. Stacked atop #2193 (and #2181).

This doesn't yet cause end-to-end-encrypted notifications to actually happen in the live app, because we're not yet setting up for the server to send them to us.

chrisbobbe

Thanks, this is exciting! Small comments below, and also:

I started reading the ZAP document and related CZO discussion: #api design > E2EE - cryptography @ 💬

I think maybe this part of the ZAP document is outdated?:

Each mobile push notification a Zulip server sends will be encrypted with a public key belonging to the intended user device. The corresponding private key will have been generated by the Zulip mobile app on the device, and will remain only on the device.

because it contradicts this part that was added later (and which is what we're actually implementing, right):

Notifications are encrypted with symmetric cryptography, using a secret key shared by an individual client (the Zulip app on a given user device) and a Zulip server.

I also noticed this part:

Server admins, realm admins, and individual users will also have the option to require all notifications to be end-to-end encrypted. The server will offer both a server-level and a realm-level setting; when either setting is enabled, no unencrypted notifications will be sent, and older clients will no longer receive notifications. The client will offer a setting; when enabled, the client will not register for notifications except through this protocol, and so will not cause older servers to send it notifications.

If that's still the case, i.e. if on server 12+ there might still be unencrypted notifications, would you scan through the TODO(server-12)s to see if they're overlooking that case?

chrisbobbe · 2026-03-03T22:18:58Z

pubspec.yaml

  path_provider: ^2.0.13
  share_plus: ^12.0.0
  share_plus_platform_interface: ^6.0.0
+  sodium_libs: ^3.4.6+4


Commit-message nit:

deps: Add sodium_libs Just `flutter pub add sodium_libs` and then fixing up the sorting in pubspec.yaml.

Looks like the "then fixing up the sorting" part is outdated; you fixed up the sorting in a separate prep commit. :)

What that's intended to mean is that flutter pub add puts the new library at the end of the list, so I moved it to the right spot.

chrisbobbe · 2026-03-03T23:41:44Z

lib/notifications/receive.dart

+    try {
+      parsed = EncryptedFcmMessage.fromJson(origData);
+    } catch (_) {
+      // Presumably a non-E2EE notification.  // TODO(server-12)


Will all notifications be E2EE at server 12+, or is it an optional feature? (looking at the TODO(server-12))

All notifications will be E2EE at server 12+ for clients that support it. So when we drop support for older servers, we can drop support for the legacy non-E2EE notifications.

chrisbobbe · 2026-03-03T23:51:58Z

lib/notifications/receive.dart

+    final pushKey = globalStore.pushKeys.getPushKeyById(parsed.pushKeyId);
+    if (pushKey == null) {
+      throw Exception("unknown pushKeyId"); // TODO(log)
+    }
+    final account = globalStore.getAccount(pushKey.accountId)!;


One or the other of these can throw in the case of a notification received for a logged-out account, right? (The Exception("unknown pushKeyId") or the .getAccount(…)!.) Do we instead want an early return with a comment, like in _onPlaintextRemoteMessage?

The ! should never throw, because PushKeys.accountId is a foreign key with cascading deletes: when an Account record is deleted from the database, any corresponding PushKey records will be too.

Good point that an unknown push key can occur if the account was logged out. I'll have it return instead.

Wrote this:

if (pushKey == null) { // Not a key we have; nothing we can do with this notification-message. // This can happen if it's addressed to an account that's been logged out. // TODO(#1764) try to unregister on logout (though this will still sometimes happen) return; }

That TODO is already handled later in my draft branch.

gnprice · 2026-03-04T02:31:09Z

I think maybe this part of the ZAP document is outdated?:

Good catch, yeah.

I think a lot of details of the protocol are out of date in the ZAP, too. I don't think I want to update all those details, as it feels redundant with the API docs. But should probably update that high-level point, to match the updated other section you spotted; and maybe add some links at the top pointing to the API docs and/or other updated sources.

I also noticed this part:

Does #2194 (comment) answer this question?

gnprice · 2026-03-04T02:36:19Z

Thanks for the review! Revision pushed.

Just `flutter pub add sodium_libs` and then fixing up the sorting in pubspec.yaml.

We'll add features to the tests' fake implementation as we start using them.

Originally this value was used in constructing the Android "intent" for the user opening the notification; effectively it was passed through to the app if the notification was opened. We haven't done that since 0fc0a0f, though, back in 2024.

This is more uniform, and closer to the behavior we'll want with E2EE notifications. This change is NFC for the handling of MessageFcmMessage or UnexpectedFcmMessage. It's nearly NFC for the remaining case, of RemoveFcmMessage. The difference is that if we receive one of those for an account that's unknown (e.g. because logged out), we'll now ignore it. Previously we would try to act on it... though it was already unlikely that would end up doing anything, because when logging out we already remove any existing notifications for the former account. (See removeNotificationsForAccount and its call site.)

This is pretty vacuous for the moment; but it'll let us make changes to this setup by editing just one place.

This will be useful when we start having these tests exercise the upcoming code paths for E2EE notifications.

This makes a pretty small simplification in itself. It'll do more when we start having these tests produce encrypted notifications.

chrisbobbe

Thanks! Just small things below, otherwise LGTM.

chrisbobbe · 2026-03-09T20:44:43Z

test/notifications/display_test.dart

 import '../api/notifications_test.dart';

+Future<Uint8List> _encryptNotification(Uint8List pushKey, Uint8List plaintext) async {
+  // Compare [PushDeviceManager.decryptNotification].


nit:

Suggested change

// Compare [PushDeviceManager.decryptNotification].

// Compare [PushKeyStore.decryptNotification].

chrisbobbe · 2026-03-09T20:47:13Z

lib/model/push_key.dart

+    // The Sodium docs say to call this before SodiumInit.init():
+    //   WidgetsFlutterBinding.ensureInitialized();


Suggested change

// The Sodium docs say to call this before SodiumInit.init():

// WidgetsFlutterBinding.ensureInitialized();

// The Sodium docs say to call `WidgetsFlutterBinding.ensureInitialized()`

// before SodiumInit.init().

I think I read too hastily at first and took "this" to mean the thing annotated by the comment, i.e. the ZulipBinding.instance.sodiumInit() call (which calls SodiumInit.init()).

Toward zulip#1764. This doesn't yet cause end-to-end-encrypted notifications to actually happen in the live app, because we're not yet setting up for the server to send them to us.

gnprice · 2026-03-10T02:26:06Z

Thanks! Merging with those fixes.

gnprice requested a review from chrisbobbe March 3, 2026 05:56

gnprice added the maintainer review PR ready for review by Zulip maintainers label Mar 3, 2026

chrisbobbe reviewed Mar 4, 2026

View reviewed changes

gnprice force-pushed the pr-e2een-decrypt branch from efb43c9 to 7a85b6a Compare March 4, 2026 02:36

gnprice force-pushed the pr-e2een-decrypt branch 3 times, most recently from 7cd294f to 1c6f4c0 Compare March 9, 2026 21:38

gnprice added 10 commits March 9, 2026 14:59

deps [nfc]: Fix sorting of deps in pubspec

4ffd672

deps: Add sodium_libs

1649d2e

Just `flutter pub add sodium_libs` and then fixing up the sorting in pubspec.yaml.

binding: Add sodiumInit, initially vacuous in tests

c9f21c3

We'll add features to the tests' fake implementation as we start using them.

notif [nfc]: Find matching account earlier still

2b6ad6b

notif test [nfc]: Pull out a helper addAccount

a878e68

This is pretty vacuous for the moment; but it'll let us make changes to this setup by editing just one place.

notif test: Insert a push key in setup

7210ea3

This will be useful when we start having these tests exercise the upcoming code paths for E2EE notifications.

notif test [nfc]: Make receiveFcmMessage helper async

69aa682

notif test [nfc]: Add helper encodeFcmMessage

4c7487b

This makes a pretty small simplification in itself. It'll do more when we start having these tests produce encrypted notifications.

gnprice force-pushed the pr-e2een-decrypt branch from 1c6f4c0 to f04af23 Compare March 9, 2026 22:00

gnprice mentioned this pull request Mar 9, 2026

Unregister device on logout; adjust when to unregister for legacy notifs #2211

Merged

chrisbobbe reviewed Mar 10, 2026

View reviewed changes

notif: Decrypt E2EE notifications!

4655cc1

Toward zulip#1764. This doesn't yet cause end-to-end-encrypted notifications to actually happen in the live app, because we're not yet setting up for the server to send them to us.

gnprice force-pushed the pr-e2een-decrypt branch from f04af23 to 4655cc1 Compare March 10, 2026 02:25

gnprice merged commit 4655cc1 into zulip:main Mar 10, 2026
2 checks passed

gnprice deleted the pr-e2een-decrypt branch March 10, 2026 02:26

	// Compare [PushDeviceManager.decryptNotification].
	// Compare [PushKeyStore.decryptNotification].

		// The Sodium docs say to call this before SodiumInit.init():
		// WidgetsFlutterBinding.ensureInitialized();

Conversation

gnprice commented Mar 3, 2026

Uh oh!

chrisbobbe left a comment

Choose a reason for hiding this comment

Uh oh!

chrisbobbe Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gnprice Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

chrisbobbe Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

gnprice Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

chrisbobbe Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

gnprice Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

gnprice Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

gnprice commented Mar 4, 2026

Uh oh!

gnprice commented Mar 4, 2026

Uh oh!

chrisbobbe left a comment

Choose a reason for hiding this comment

Uh oh!

chrisbobbe Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

chrisbobbe Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gnprice commented Mar 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chrisbobbe Mar 3, 2026 •

edited

Loading

chrisbobbe Mar 9, 2026 •

edited

Loading