Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Encrypt push notifications end-to-end from Zulip server #1190

Open
gnprice opened this issue Sep 19, 2017 · 13 comments
Open

Encrypt push notifications end-to-end from Zulip server #1190

gnprice opened this issue Sep 19, 2017 · 13 comments

Comments

@gnprice
Copy link
Member

gnprice commented Sep 19, 2017

Currently the private data in a Zulip push notification (sender, stream name, message body) is encrypted over the wire, but handled in plaintext within the Zulip notification forwarder and the Apple or Google notification service.

This is consistent with standard practice until at least last year; but last October Apple introduced in iOS 10 a way to have the app's code run to adjust a received notification, which opens the option of encrypting the data all the way to the app. That'd be neat, and doubly so because for a Zulip server other than zulipchat.com, there are two third parties involved in handling the notification: Apple or Google, and Kandra Labs.

Quoting my comment at zulip/zulip#6364 (comment) (on a PR specifically about iOS):
"""
A bit of web searching tells me that as of 2016 (and iOS 10), there is actually a way to have your app's code invoked when a notification is delivered! Docs here: https://developer.apple.com/documentation/usernotifications/unnotificationserviceextension

It's not well documented. The APNs payload reference @kunall17 linked above doesn't mention the relevant key, and claims that all keys not mentioned "are ignored by Apple". I'm a little dubious of how well-supported it will be, like whether the OS will reliably actually call the extension and how much time it'll give the extension to run. As part of Apple's obsession with battery life for iOS, they've always been stingy with every form of background processing for apps, which is why push notifications without this API have always been self-contained data that the OS would interpret without consulting the app.

I think my inclination would be:

  • For now, just include the message content like every other app that includes content in notifications must have been doing until last October.
  • Later, improve on that to keep the data encrypted end-to-end from the Zulip server to the device -- but by including it in the message, encrypted with a key the app has previously exchanged with the server, so that no network fetch is required. That won't make it for 1.7, but I think a natural way of engineering it will allow it to work for any given user as soon as both their client app and their server run a version that supports it.

"""

I believe something similar is also possible for Android, though I haven't read up on it.

@gnprice
Copy link
Member Author

gnprice commented Sep 19, 2017

Quick supplement:

  • iOS 10 is now on 89% of iOS devices known to the App Store: https://developer.apple.com/support/app-store/ So almost all iOS devices will be able to enjoy this feature. We'll need to make sure to behave robustly where it's absent, which we might accomplish at the same time as handling the case of old versions of the app.

  • A couple of SO threads as small cautionary tales where developers are hitting mysterious failures when using this feature -- hard to sort out how much of it is their fault in avoidable ways, and how much is likely to cause frustration for anyone: here, here, here. This one is good for having a list of pitfalls the asker eventually figured out.

@borisyankov
Copy link
Contributor

I did create a 'Security' project to group all related issues.
This should be a priority area for us, as an open-source project, and as an organization proud of our technology.

@julian-weinert
Copy link

Thanks for the hint to this in your mail, Greg!

Reading through your original proposal I see the current idea would be to add a notification extension. This is indeed a nice solution, but I think it could be done much more simple by loading the data from the on-premise server directly.

There would be no need for an extension or further de-/encryption. When the server sends a "content-available" key within the notification payload, the app itself starts in background mode and gets about 30 seconds to run.
https://developer.apple.com/library/content/documentation/NetworkingInternet/Conceptual/RemoteNotificationsPG/PayloadKeyReference.html#//apple_ref/doc/uid/TP40008194-CH17-SW1

This way it could be done quite quickly without introducing a lot of new code.

@borisyankov
Copy link
Contributor

@julian-weinert this needs to work on Android too. (and older versions of iOS and older Android).
Do you know if this would work everywhere?

@julian-weinert
Copy link

@borisyankov For older versions of iOS yes.
The original proposal would work from iOS10 upwards, my solution should run down to 8.
Probably not down to the oldest versions, but without pushing new features, we would still be using parallel printer ports.

Unfortunately, I don't know, how Android works.
But if it's not possible on Android, then the original approach wouldn't work either.
Actually I think my approach would be more Android friendly.

After searching the internet it seems as it is indeed possible, although not many people are talking about it. I could also imagine I simply don't know how to google for android topics correctly ;)

@gnprice
Copy link
Member Author

gnprice commented Jan 17, 2018

Thanks @julian-weinert ! The reason I think I'd prefer to include the details with the notification payload is that it allows the notification to render without any additional network requests. That might not be a big deal when on a fast, reliable Internet connection; but a lot of places a phone can go don't always have a good connection, and I think it's important to design apps to work gracefully in that case.

Anywhere we're able to run code to contact the server before a notification is rendered, we should be able to decrypt something as well. (We do need a key to decrypt; but we also need some kind of secret to authenticate to the server.)

Re iOS version compatibility: new iOS versions tend to get adopted much faster than new Android versions, so I'm not concerned about having some features of the app only work on iOS 10+. Looks like the latest data from the App Store shows 92% of devices on iOS 10+.

@borisyankov
Copy link
Contributor

@gnprice I have discussed this with Tim before, the percentage game is dangerous.
Looking at 92% support is low, if you think about it. That means 8% of people can not use the app. This is not 8% of teams, but of individual users. And a team is comprised of multiple users.
That might mean almost every organization having several people with incompatible devices.

@gnprice
Copy link
Member Author

gnprice commented Jan 17, 2018

@borisyankov I'd definitely agree with that when it comes to the app working at all, or core functionality of the app.

I think it's different for features that are nice to have but the app still works fine without them. In this case, like I said above, we'll need to make sure to behave robustly where this API is absent, and we might accomplish that at the same time as handling the case of old versions of the app.

@borisyankov
Copy link
Contributor

Cool. I agree with you too. So something like 'to take advantage fully of our security features, use iOS 10+ and Android 7+' sounds reasonable.

@timabbott
Copy link
Member

Just wanted to add that I'm 100% on board with that plan as well.

@umairwaheed
Copy link
Member

Let me add a v2 (version 2) endpoint for registering a device. Any device registered through this endpoint will be expected to work with encrypted notifications, while any device registered through v1 endpoint will continue to work as before. I think this is the only thing left in zulip/zulip#8076 for a seamless transition.

@zulipbot
Copy link
Member

Hello @zulip/server-notifications members, this issue was labeled with the "area: notifications" label, so you may want to check it out!

@gnprice
Copy link
Member Author

gnprice commented Aug 20, 2019

A couple of links for reference, as I just ran across them:

  • Google's Android org offers strong guidance that the plan of including encrypted content in the FCM payload is the right one, and we should avoid any design that would mean making network requests before we can show the notification to the user. So that's in line with our thinking above.

  • A couple of engineers on a Google privacy team have published a library, Capillary, for managing end-to-end encryption for notifications.

    • There may be some useful ideas to be had from reading this. Unfortunately I don't think the actual software will be particularly suitable to use, for a few reasons:
    • It's only for Android, so we'd need something else on the iOS side. OK, it's likely we'll end up settling for that anyway, but...
    • On the server side it's only for Java, with no guidance on how to write a new server-side implementation to interoperate with the client side.
    • Also, wait, Java? Literally Java, not Kotlin. For something intended for the Android ecosystem, and newly written in 2018 with no obvious legacy-codebase constraints. That is a real bad sign for the authors'... with-it-ness, and the quality of their implementation.
    • Then there's just very little sign of anyone using it in the year since it was published -- very little activity on the repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants