JavaJens / TextSecure

A secure text messaging application for Android.
GNU General Public License v3.0
71 stars 9 forks source link

PullRequest to upstream #21

Open JavaJens opened 9 years ago

JavaJens commented 9 years ago

I'd like to use this issue to discuss any changes required before submitting a pull request to upstream.

dev-mb-zz commented 9 years ago

As far as I followed the discussion moxies biggest concern was battery drain when using websockets instead of gcm. To have some arguments it would be good to have numbers available to compare gcm and websockets versions in terms of relative battery drain in same usage scenarios.

Are there any reliable procedures for measuring battery drain in dependence on idle/typing/sending/recieving activity? Sadly this doesn't seem to be trivial to me.

Isgar commented 9 years ago

In my experience having gapps installed is a huge drain on battery of itself. Apps using GCM thus don't have additional drain. Websocket TextSecure added a big drain on my gapps-free device mostly since there was nothing waking the device up before.

So to properly compare it, you'd have to have a clean install with and without gapps/websocket. Even then it is questionable how these numbers compare to real life scenarios.

dev-mb-zz commented 9 years ago

Thats a good point, I observed that Gapps used a lot of battery as well. No this consumption moved to Textsecure. But in general battery lifetime seems to be in same orders of magnitude (2-3 days depending on usage - MotoG fist generation 2years old).

relyt29 commented 9 years ago

Regarding battery life, lets gather some statistics, in an organized fashion. The problem with effective statistics is that you need to be able to effectively control for your environment.

As for presenting the pull request, probably what we will need to do is as you you mention in #15, although again, the problem is staying current with upstream. What I would suggest is maintaining the status quo, doing the code review on websocket-reborn, and then when a specific goal has been achieved (but I don't know what that is) we take all the diffs of websocket-reborn, rebase, and apply them in one commit, and submit the PR. Then inevitably, the PR will sit there for a week and a half until somebody upstream gets a chance to look at it, by which point it will be behind again. But at least at that point, we'll have feedback to incorporate back in, and do it all again until its finally "good enough."

schachmat commented 9 years ago

So did we do enough testing to try another PR to upstream? The changeset is not too complex at least.

relyt29 commented 9 years ago

EDIT: I didn't see someone did do a battery test for #22. How many of those are good enough before we have enough data? I think #22 needs to be resolved first, so that we have something to bring to the table with regards to performance stats, better than "I've been using this for 2 months with both data and wifi, and my battery life has been fine, everything works great, I haven't experienced any bugs" even if its true (which it is) its still only personal anecdote.

Also, some more code review would be nice, to make sure its up to spec. Moxie has really strict code quality requirements, and this is maybe a few hundred lines of code. Also there would need to be a PR made in tandem with libtextsecure-java for the fetchesMessages support.

If we could get a list of how many people have actually read all of the changes and checked that they're sane that could be helpful to get an idea whether or not we think we're ready to merge. like a Google spreadsheet of github users who have read and checked off on it. @JavaJens what do you think of that idea?

Also javajens probably will have to rebase off master before submitting, and get rid of all my "merging upstream 2.2blah" commits and whatever else noise

JavaJens commented 9 years ago

Rebasing is no problem. It works without merge conflicts and I've removed the build flavor and config stuff, so it is even less code.

However, I agree with @f41c0r that some more code review would be good. We could use the wiki in the Repo for that?

Regarding libtextsecure: I once opened a PR where moxie said it will come: https://github.com/WhisperSystems/libtextsecure-java/pull/2 Now there is another PR with the same changes, but no comment https://github.com/WhisperSystems/libtextsecure-java/pull/5

I will try to get my old phone running again for some battery tests. Do you think we can use espresso for automating the testing?

schachmat commented 9 years ago

The last two points are clear of course. And the current changes are actually really not that many ("85 additions and 27 deletions") and if you leave out the applicationId from the Manifest and the build.gradle stuff, which will not be needed upstream, it's even less.

For comparing the battery drain, I cannot help unfortunately, since I do not have and do not want gapps on my phone. However I have a proposal for people who have a >=Lollipop phone with gapps and can afford to not use it for a while:

  1. fully charge the phone
  2. leave it at your desk untouched for 24 hours
  3. during that period make sure the phone gets 20 textsecure messages with at least 5 minute gaps in between (either from a second phone or just ask a friend)
  4. after the 24 hours go to the battery settings, select TextSecure and write down the values or take a screenshot
  5. Also take the dump with Battery Historian

Do this once for the version with gapps/GCM and once only with websockets. Then we have some data which is actually comparable and since the process in which it was collected is specified, the data can actually be discussed. Of course the two values of 24 hours and 20 messages are arbitrarily choosen and can be changed.

ghost commented 9 years ago

Perhaps a dumb commented, but I'm assuming ios doesn't rely on gcm. So how does signal handle this?

paride commented 9 years ago

I would say using the Apple Push Notification Service:

https://en.wikipedia.org/wiki/Apple_Push_Notification_Service

ghost commented 9 years ago

@legovini What about Telegram, Whatsapp, etc on Android?

relyt29 commented 9 years ago

WhatsApp is closed source, probably also uses GCM, since its better for battery and whatnot Telegram is open source, but has a dependency on Google Play Services so is almost certaintly using GCM.

edit: I stand corrected see @legovini's post

paride commented 9 years ago

Telegram is able to work without GCM, and the version on f-droid supports only this way of operation (as far as I understand from the description of the package). It has no major drawbacks. I'm not sure about how it works, but for sure websockets are the right way to implement this functionality.

relyt29 commented 9 years ago

I stand corrected, however: https://security.stackexchange.com/questions/49782/is-telegram-secure

ghost commented 9 years ago

I'm currently on CM 12.1 without Gapps. Both Whatsapp and Telegram work fine :) why can't we do the same? :p

ghost commented 9 years ago

Telegram is Open-Source so I suppose we could have a look at it?

ghost commented 9 years ago

@f41c0r Indeed, telegram does store messages so that one can acces then on multiple devices. If you prefer end to end encryption you can use the secret chat feature which won't store the messages on telegrams servers and it's encrypted end to end. There's been a lot of negativity about telegram but really, It's really not as bad as people think. We could definitely check out their source and see how they handle things.

h-2 commented 9 years ago

I use Conversations, Kontalk and Telegram, all without Push and I get 2 days of battery life with my OnePlus One. Kontalk is even in "always-on" mode where it permanently keeps a connection. The main issue with not having push is that multiple applications poll or keep-alive at different intervals. If all of the mentioned applications where to do so every 6 minutes of the hour exactly (for example) the phone could at least sleep in between. That having been said Web-sockets allow for "always-on" with much lower keep-alives (right?). So JavaJens's approach is already much better.

ghost commented 9 years ago

Perhaps that would be a nice feature for Android. Allowing users to set a refresh interval in android itself. That way it could sleep said interval and allow all apps to refresh at once. This interval can then also increase/decrease for battery saving.

schachmat commented 9 years ago

Just use http://developer.android.com/reference/android/app/AlarmManager.html#setWindow(int, long, long, android.app.PendingIntent) for that. Enforcing a global refresh interval would restrict apps needing a more frequent wakeup.

h-2 commented 9 years ago

In any case thanks for still working on this. It is relevant to a lot of people I know!

ghost commented 9 years ago

So I came across a few articles that somehow suggest governments can't read whatsapp as it's end-to-end encrypted. They did indeed work with open whisper systems but they never documented this.

http://uk.businessinsider.com/whatsapp-snapchat-encryption-isis-lord-king-2015-1?op=1

http://readwrite.com/2015/01/13/david-cameron-encryption-messaging-apps-imessage-whatsapp-snapchat

I'm assuming whatsapp's encryption isn't an obstacle for the NSA is it? There's no way they have an actual good implementation of end-to-end encryption and the feds be ok with that.

P.S sorry if this is completely off topic

JavaJens commented 9 years ago

IIRC correctly this is only on the Android version and it is not transparent to the user if the encryption is used or not: http://www.heise.de/ct/artikel/Keeping-Tabs-on-WhatsApp-s-Encryption-2630361.html See this blog post for the OWS side of things: https://whispersystems.org/blog/whatsapp/

But please, bring it back to topic :wink: Maybe the (un)official mailing list is a better place.

ghost commented 9 years ago

Plus the fact that there's no fingerprint checking so there's no way to tell if there is a mitm attack.

Where exactly can I find the mailing list?

h-2 commented 8 years ago

I see that we now get LibreSignal builds, but that RedPhone registration is still disabled. Is this something that is planned to work with Websockets, as well? Probably a unified patch will be necessary to be accepted upstream, now that the apps are merged, right?

ghost commented 8 years ago

Wow, this endeavor has been going on quite a while. I admire the determination all around, thank you.

Do we think this has a chance of accepted by @moxie0? And 3rd party distribution would be accepted. It seems he's been doing everything possible to stop that. It'd be a shame if all this hard work were to go to waste.

iexos commented 8 years ago

They can't really do anything about 3rd party distribution as long as it doesn't violate trademark.

But if this fork really is not going to be merged, it is worth considering setting up a seperate server with federation to Open Whisper Systems. Apart from facilitating goodwill upstream, this prevents WS Signal to break should they decide to close/change the websocket API.

h-2 commented 8 years ago

I think distribution and websockets are two different issues and should be treated as such. Let's try to tackle websockets first and get that upstream. Whether there will official TextSecure builds for F-Droid? I doubt it, but making a lot of commotion about third party builds now won't help us to get websocket-support merged...

ghost commented 8 years ago

I think distribution and websockets are two different issues and should be treated as such.

According to Whisper System's statement they're directly related to the issue of supporting devices without Google services. That's the whole point of adding websocket functionality, correct?

it is worth considering setting up a seperate server with federation to Open Whisper Systems

@iexos I think this is the most realistic option. It's closing in on 3 years and still no reliable solution.

h-2 commented 8 years ago

According to Whisper System's statement they're directly related to the issue of supporting devices without Google services. That's the whole point of adding websocket functionality, correct?

Yes and No. The point of websockets is supporting non-GCM platforms, but that doesn't mean they support F-Droid as distributions model or anything like that. I think they might accept the patch just as a basis for other clients (Chrome-Plugin...) and not offer any other Android packages. Or they might offer self-updating, self-signed APKs. My point is that since I know Moxie has a particular dislike for F-Droid, I don't think we should associate Websockets too strongly with F-Droid-distribution, because it will just lower the chances of the patch being accepted.

it is worth considering setting up a seperate server with federation to Open Whisper Systems

@iexos I think this is the most realistic option. It's closing in on 3 years and still no reliable solution.

maybe @JavaJens can comment on this? I thought the plan was still to try and get the patch upstream, or not? Also note that OWS Server does not federate "just like that", but that they have to explicitly whitelist the other server which means they are not going to do it if you make them angry... Also it was reported that federation (with the only other server by CM) does actually not work too well, but I don't have up-to-date information on that.

iexos commented 8 years ago

Also note that OWS Server does not federate "just like that", but that they have to explicitly whitelist the other server which means they are not going to do it if you make them angry...

moxie said he prefers we don't use their server, so I would guess he would at least be more open to federation? If it is technically viable, of course. In any way, splitting the userbase is not an option.

ddast commented 7 years ago

@JavaJens Is there still interest in trying to make a pull request? I recently had a look at your implementation and made some minor changes, such as changing the registration process in a way upstream wants it and deactivating voice since it does not work withough GCM. To keep the diff minimal I removed things like the websocket flavor. Do you want me to make a pull request for your repo such that we can discuss what is still missing?

mimi89999 commented 7 years ago

@ddast Yes, there is. Look at https://github.com/LibreSignal/LibreSignal/issues/43 . There is even a $210 bounty on it. (https://www.bountysource.com/issues/35722527-create-proper-pull-request-to-add-libresignal-s-websocket-support-to-ows-signal).