SafeSlingerProject / SafeSlinger-iOS

Source code for iOS platform client SafeSlinger
MIT License
6 stars 2 forks source link

Push token and keys storage logic #52

Closed brnunes closed 9 years ago

brnunes commented 9 years ago

I was testing the app and found a problem on the logic behind the push token and the storage of keys. I installed the app on the devices A and B, created the keys for each of them and exchanged the keys between them. I exchanged some messages and it worked. Then, as if I had forgotten my password, I created another set of keys on device A, and exchanged keys again with device B. When importing the contact, device B did not save the new keys ("0 contacts imported"), while device A did. I think this has to do with the push token being the same as the old entry for A, which is already in the database (the push token 'ptoken' is the primary key of the table). Now A has the correct keys but B does not. B sends messages using the old keys. The messages are delivered but A cannot decrypt ("Signature verification failed"). When A sends a message to B, B doesn't recognize the sender and can't decrypt ("Unable to match public key to private key in crypto provider"). If A enters with his old keys, A receives the messages from B and can decrypt them, as well as send messages to B that B will be able to receive and decrypt. The problem was found on versions 1.8.0 and 1.8.1. I don't know how this is implemented on Android, but I think that using a different push token for each set of keys created on the device should solve this problem. Then maybe the key storage logic should change.

mwfarb commented 9 years ago

When I built the Android key database, I was planning for key-drift, but I also found during testing that there was a surprisingly high rate of push registration drift as well in Android push services. So early in the database design I allowed that the combination of push registration id + public key would make one unique entry in the database. I think the push registration id for APNS/UA ended up being much more stable in iOS testing which is why its the only unique key in the iOS key database.

Perhaps iOS is much closer to Android in this respect and the unique key should be public key + push registration. Here are the scenarios in which I found either element changed while the other stayed the same:

Each scenario is a legitimate use case we should allow for updated storage. Unfortunately, we cannot just request a new push token, it is the OS environment that determines when a new push registration token is assigned to a new/old device.

tenmalin commented 9 years ago

Current iOS database holds recipient contact by push token index only. If the user regenerates his own public key by making another passphrase, the recipient's app should update the entry after the user slung his new key. Therefore, device B should overwrite the new key to its database since the key for a particular token has been updated. I guess the problem happens when app tries to update its database.

I knew the better design is making combination of push registration id+key id as an index for token database. I did not change it for a while since I am afraid that I might make the database format inconsistence with old app users. But I think we should update old apps for 1.8.1 pushes, to avoid such kind of bugs after new version releases.