signalapp / Signal-Android

A private messenger for Android.
https://signal.org
GNU Affero General Public License v3.0
25.7k stars 6.17k forks source link

Websocket support #1000

Closed JavaJens closed 7 years ago

JavaJens commented 10 years ago

As #127 is a little overloaded and goes beyond the technical, I thought to open up this issue to gather implementation details and coordinate efforts for this.

Support for WebSockets is already present on the server, so the idea is to use WebSockets on Android as well (for now instead of a possible more suited protocol like MQTT)

@geileszeuch mentioned a guide on how to use websockets in native apps: http://www.elabs.se/blog/66-using-websockets-in-native-ios-and-android-apps

However I don't think this would be a good guide as it doesn't talk about Wake/Sleep and connection loss. This article list several issues one has to deal with in a mobile enviroment: http://dalelane.co.uk/blog/?p=1599 I believe the approach mentioned here http://stackoverflow.com/a/16602970 to be reasonable, if I find the time I will give it a try.

I guess for developing this one has to run an own server, as WS is disabled per default.

So please, if you have a question, concern or are already working on this: comment!

JavaJens commented 10 years ago

@SecUpwN I don't know if and most importantly when these changes might be integrated in the main version. Right now the branch is way to buggy and the main server doesn't support WebSockets yet. Also the my fork is highly insecure! It might have log messages that leak private data and doesn't use secure Websockets.

Therefore DON'T use this branch as a replacement! This is only for developing until the branch is more stable and the official server supports Websockets.

grote commented 10 years ago

To clarify what the F-Droid version is: It uses a separate repository from the official F-Droid one that can be installed as an add-on and it just contains the apk build from the feature branch source. So it is only the test version and can therefore also be installed alongside the official TextSecure and is not meant as a replacement.

This is purely to distribute the test version to a larger group of people for testing purposes. In order to get the new version show up in F-Droid, the version code would need to be increased by one though.

kirschner commented 10 years ago

Thanks @JavaJens for your work!

I am using the build by @grote . I can receive messages from @grote and one other user but it seems they do not receive my messages. Furhtermore there is a longer delay in messages.

Is there anything specific I can test to help you solving the problems?

h-2 commented 10 years ago

I also exerience a delay with textsecure messages, to me it looks like the clients pulls from the srver every x minutes. Is this the case? If yes, could the behaviour be changed to where it holds a continuous connection for as along as the screen is turned on (i.e. the device is being used) and switch to polling every x minutes once the phone is in sleep? Ideally this could be configured in the options!

I can also confirm that automatic key exchange and encrypted SMS work with regular TextSecure users (don't now how exactly, but it does).

JavaJens commented 10 years ago

@h-2 The way the websocket connection works is the following: When TextSecure is opened or Internet connection is established, a websocket connection to the server hosted by @TheBlueMatt is opened. This connection is always kept open! Every ~50s a heartbeat is sent to the server and replied by a Pong. If a message arrives it should therefore be immediately delivered.

There are three scenarios in which a delivery could be delayed: 1) No connection to the Internet->Delivery on connection 2) Dropped connection -> Connection should be re-established in at most 50s 3) Error on the server side that denies connections -> The phone tries to reconnect every 50s, this is bad and will be replaced by an exponential back-off soon.

Using this method should be effective as the phone can sleep during the Ping interval and will be woken on incoming messages or by the timer. However the issues we are currently experiencing are different, at least from my point of view, as I seem to have a working( i.e. ping/pong messages arrive) connection, but the messages are delayed anyway. As the same issues are experiences in the Chrome extension, I could imagine the server being at fault here, but this is just a guess.

As far as debugging goes: The best setup would be if you either use several devices or have a second communication channel to the other party so you know when messages are sent. When sending or waiting for a message always grab a debug log using logcat. This way any errors on the phones can be seen.

I have seen several errors of MAC mismatches, maybe you experience the same? Also are PONG messages received in a time where other messages should also be received? Do the PING messages work for you?

@grote The error you described seems to me to be of a different nature. Without further research I think the "receiver" in this case is more of an android Intent/Service receiver. This could be due to the different package name I used. I will look further into this if I find the time.

Lastly, @TheBlueMatt do you experience similar issues while developing the Chrome extension? Maybe you can chip in with your knowledge.

grote commented 10 years ago

I now updated the version in the test repository for F-Droid to the latest commit:

http://grobox.de/fdroid/repo

@JavaJens I managed to fake a higher version code, so you don't need to bother with it.

Thanks for all your work! I tried sending you and other people messages, but they seem to never arrive at the moment. Thanks also for looking into this "Receiver not registered" issue. Could this maybe be related to the fact that I did not make TextSecure my default SMS app? Otherwise, the changed package id might indeed be a clue, but then you should see this as well.

TheBlueMatt commented 10 years ago

re: getting logs, I have no problem providing any logs you want, but it may be best to just coordinate a time where I can provide logs immediately when someone is testing...ping BlueMatt on freenode or email.

eighthave commented 10 years ago

Nice to see progress on this, and to see an FDroid developer repo in action. For a little extra security, it is possible to include the signing key fingerprint in the URL: http://grobox.de/fdroid/repo?fingerprint=28e14fb3b280bce8ff1e0f8e82726ff46923662cecff2a0689108ce19e8b347c

And of course QR Codes are sometimes handy: grobox de-fdroid-repo

moxie0 commented 10 years ago

@JavaJens @grote I would prefer it if you'd stop passing around APKs via F-Droid. I think the only people who should be using this build are people who are capable of building themselves.

Another thing to consider is how you plan on eventually integrating this work. A PR with an enormous diff will probably take a very long time to get merged, so if possible I would recommend a more incremental approach.

JavaJens commented 10 years ago

@moxie0 I know how you feel about that, thus I haven't promoted that. I totally agree with you especially regarding the many security issues with the current status.

About integrating. I think it will not be a big diff, as I keep integrating the upstream changes. Plus there aren't too many changes anyway.

However there are still several open issues.

Maybe you can give me a hint on how you'd like to continue ?

grote commented 10 years ago

@moxie0 Please note that the apk is not distributed to all F-Droid users, but only to the people that find this discussion and add the test repo manually. Then, there's a big fat warning about the test nature and insecurity of this build in the app description.

I personally think that this is good enough and widens the group of people I can test with. Considering the open issues we face, this is appreciated.

There's an open question pending on the mailing list which you might know an answer to.

moxie0 commented 10 years ago

@JavaJens If the PR is going to be small don't sweat it. I just wanted to give you a heads up about doing some incremental changes if you expect it to be large.

@grote Unfortunately people have started promoting it elsewhere, which I think will cause problems.

I think that if you want to do testing, it might be more effective to either run 1% tests across the installed TextSecure base, or to distribute APKs which test specific functionality but aren't full TextSecure builds. For instance, if you want to test the effect of websockets on battery life, I think it would be more effective to put together a small APK which only encapsulates websocket functionality, measures connectivity, measures the effect on battery, and packages up all the device info, network type, etc, into results which are reported back to you. I think having people manually try to do the same with a full TextSecure build will result in more unknowns and more anecdotal results that are more difficult to quantify.

xylo commented 10 years ago

@moxie0 Providing a test repository with pre-compiled binaries is a very common solution to find testers (see for instance debian, synology, mozilla, ...). Moreover it is the best solution you can do if you really want to find testers for this feature, since it addresses exactly the people that do not have the Google Market installed and would be therefor unable to install TextSecure at all. If you force all testers to compile the app themselves you'll get a lot of testers with outdated versions since no tester is going to rebuild the app every day.

Furthermore, the people that use the F-Droid repository posted by eighthave know exactly what they do and can definitely distinguish between official versions of TextSecure and unofficial testing versions hidden behind a personal F-Droid repository that one needs to manually add before being able to install the test version.

Regarding your suggestion of running 1% tests across the installed TextSecure base I'm very sceptical. First, you need to find enough volunteers for testing unless you plan to secretly choose some of your stable users as guinea pigs (I hope you are not doing this). Second, those users are more likely not to be able to distinguish between a testing and a stable version than those that came to this thread, read about the problem and installed the app from a special test repository.

georgkrause commented 10 years ago

Well, i want to test this app, but i dont know how to do it. I installed the app, but there is the warning i shouldn't enter private data, and i agree with that. But now i dont have any contact for testing, which is the recommended way?

JavaJens commented 10 years ago

Right now testing the app has pretty much halted. The websocket implementation seems to work quite well as indicated by other tests, but there are issues with properly connecting to the server that need to be solved before picking user tests up again.

I am working with @grote on debugging some of these issues, but my time is very limited right now.

Nevertheless the way you could test is the following: Install TS-Websocket on two devices and add each other. Then you could message between these devices. The Server is a different one than the regular TS server therefore none of your friends are there.

You also should be able to communicate with the chrome extension, but I also had severe issues with that.

Good luck :)

2014-07-09 18:35 GMT+02:00 Georg Krause notifications@github.com:

Well, i want to test this app, but i dont know how to do it. I installed the app, but there is the warning i shouldn't enter private data, and i agree with that. But now i dont have any contact for testing, which is the recommended way?

— Reply to this email directly or view it on GitHub https://github.com/WhisperSystems/TextSecure/issues/1000#issuecomment-48499671 .

geileszeuch commented 10 years ago

The Telegram app for Android also introduced a GCM free Notification Service. I didn't check whether it's based on websockets, but I can tell that it works perfectly and uses almost no battery. Maybe this can help somehow. Here is the source code: https://github.com/DrKLO/Telegram

JavaJens commented 10 years ago

We now have support for the Staging Server! There you have to use your real phone number and get a verification code.

There are still some things left to do, but it seems usable right now.

@moxie0 I made the following change to the PushServiceSocket https://github.com/JavaJens/TextSecure/blob/0ae8a8c0e8b0a65733eda7619fb34999bd8d0989/library/src/org/whispersystems/textsecure/push/PushServiceSocket.java#L429

Could you think of any side-effects from changing that from private to public static? Also how does one delete/unregister a non GCM client?

moxie0 commented 10 years ago

Could you think of any side-effects from changing that from private to public static?

That wouldn't make it pass code review. We're sticklers for correctness.

JavaJens commented 10 years ago

So what do you suggest? Create an interface that handles the certificate pinning on a system level?

Or should we move that to one of the utility classes?

ddorian1 commented 10 years ago

Thanks for your work! Are the google services still needed for registration? I have build your branch and installed it on my phone without google apps. When I tried to registry for push, I received the verification code and the phone generates the keys, but afterwards it complains to be not able to connect to the server.

JavaJens commented 10 years ago

Mhh, thats strange. @grote is using the branch also without Gapps. There might still be some issues, but it should work in principle. Maybe you could grab a debug log of the exact error message?

2014-08-07 20:27 GMT+02:00 Johannes Schwab notifications@github.com:

Thanks for your work! Are the google services still needed for registration? I have build your branch and installed it on my phone without google apps. When I tried to registry for push, I received the verification code and the phone generates the keys, but afterwards it complains to be not able to connect to the server.

— Reply to this email directly or view it on GitHub https://github.com/WhisperSystems/TextSecure/issues/1000#issuecomment-51511958 .

georgkrause commented 10 years ago

i have the same problem and logged it. I don't have a PC at the Moment, but i can send the log file per mail or post it at the weekend.

ddorian1 commented 10 years ago

Here is my log: http://hastebin.com/hunoruwoso

JavaJens commented 10 years ago

Thanks for the Log. Looks like the same issue that @grote had. Could you try to delete all your app data (via Settings->App) and re-register? Just if you don't mind deleting the data of course ;)

ddorian1 commented 10 years ago

Deleting the app data didn't help, but this fix works for me: https://github.com/ddorian1/TextSecure/commit/17d02802c65374b01a87f2209dbe2e2625b8d1b2

ddorian1 commented 10 years ago

It seems that isUserRecoverableError returns true if it is possible to install gcm on your device (e.g. on cyanogenmod). So I would suggest to change the code to https://github.com/ddorian1/TextSecure/blob/d1147fce336e6310980bdb4277a61d78a9200f61/src/org/thoughtcrime/securesms/RegistrationActivity.java#L182

JavaJens commented 10 years ago

@ddorian1 I thought the same about the user recoverable deal.

I'll look into your changes, but on my mobile they look like they are not needed, as usually the error should not occur if we dont force to disable gcm.

I can't reproduce the error after deleting my prefs. Maybe you could check the error log again after deleting the preferences?

I know that voice registration doesn't work right now, but the rest should.

Thanks for your help!

haffenloher commented 10 years ago

When I tried to registry for push, I received the verification code and the phone generates the keys, but afterwards it complains to be not able to connect to the server.

FYI, I can reproduce this problem using a fresh build of @JavaJens' feature-1000 branch. As I did a factory reset of my device some days ago, this was the first ever TextSecure install on this Android system, so there shouldn't be any data from previous installations.

JavaJens commented 10 years ago

That's too bad. I will do some debugging next week to see if I can find the root cause here.

I think that disabling the check altogether is bad, as there might be occasions where this would be a genuine error.

@ddorian1 @brumsoel Do you have Gapps installed? Maybe the error occurs because we force it to not use GCM although it could?

Sorry, I am a little tied down right now, but will look into it asap.

ddorian1 commented 10 years ago

No, I don't have gapps, but I use cyanogenmod, so gapps can be installed. As I see it, that makes the error 'user recoverable' and I will not be able to use sockets with the check. Another solution may be to ask the user wether to use gcm or sockets when its user recoverable, but I think that's not realy user friendly.

ddorian1 commented 10 years ago

One last thought: From the documentation it seems to me, that isUserRecoverableError only returns true, if either gcm is not installed or disabled in the settings. If it's disabled, the user has decided to do so. And if it's not installed, then it would be more convenient to silently fall back to sockets then to prompt the user to install additional software. So in my oppinion the check is not necessary.

haffenloher commented 10 years ago

Do you have Gapps installed?

Nope, no Gapps installed here either.

haffenloher commented 10 years ago

Here are some results of my tests (using the registration fix by @ddorian1):

Later on, my friend reinstalled his phone, now with play services. I kept using the websocket client. Now, my client's push messages aren't received on my friend's client, while he's only offered to send me (encrypted or plain text) SMS.

Are these issues already known?

JavaJens commented 10 years ago

Hey, thanks for testing. To address your problems:

So when testing, make sure that you both are using the same server, please report back then what issues you still have. Thanks a lot!

2014-08-13 17:34 GMT+02:00 brumsoel notifications@github.com:

Here are some results of my tests (using the registration fix by @ddorian1 https://github.com/ddorian1):

  • Communicating with someone who has the official play store client installed only works through (encrypted or plain text) SMS, push messages are not offered as a choice on both devices
  • Communication between two websocket users over the data channel works, but with a strange side effect: The sender of a message immediately receives an empty, unencrypted reply. The other client does not display the empty message it (apparently) just sent. See this screenshot https://cloud.githubusercontent.com/assets/264472/3899484/55da6482-227c-11e4-9134-b4e16f1a7603.JPG .

Later on, my friend reinstalled his phone, now with play services. I kept using the websocket client. Now, my client's push messages aren't received on my friend's client, while he's only offered to send me (encrypted or plain text) SMS.

Are these issues already known?

— Reply to this email directly or view it on GitHub https://github.com/WhisperSystems/TextSecure/issues/1000#issuecomment-52065301 .

haffenloher commented 10 years ago

The official play store client uses a different server (unless you changed the server in your built). That is why you don't "see" each other.

Woops, I wasn't aware of that, sorry. Thanks for pointing it out. I will try it using the official server then.

JavaJens commented 10 years ago

I just merged the latest upstream master. However the empty messages are still there and a problem of the WebSocket message handling. Maybe @moxie0 can shed some light on why encrypted "empty" messages are sent?

haffenloher commented 10 years ago

Great, thank you! I just updated my build and will report back if I encounter any issues.

... shed some light on why encrypted "empty" messages are sent?

Just FYI, the empty auto-responses don't seem to be encrypted (no lock icon is shown for them while every regular message has one).

JavaJens commented 10 years ago

@brumsoel I think that's only a UI deal, as on the wire it seems encrypted. However I only had one quick look...

I will look into the problems with the registration tomorrow, however I can only test on the simulator without play services, as I have play on my device. So if anyone of you could use the debugger on a real device, that'd be great :)

TheLastProject commented 10 years ago

@JavaJens I have @grote's F-Droid repository, but it doesn't seem to have been updated since August 9th. If there is a newer version available somewhere (or if @grote would update his repo) I would happily test and provide a logcat. My device is a Samsung Galaxy Gio running a version of PAC-Rom without Google Play.

patcon commented 10 years ago

@grote fyi, your f-droid repo has the android.hardware.telephony feature set, so it's not possible to test from a wifi tablet. Would be great if that was removed for the next apk :)

Context: https://botbot.me/freenode/fdroid/2014-08-24/?msg=20312111&page=3

JavaJens commented 10 years ago

The best would be if you compile directly from source

rehans commented 10 years ago

I just wanted to test and debug. I assume it is still the case that I cannot send messages to my contacts using the TextSecure client from play store, right? Looking and Release.java the client seems to connect to a websocket URI of whispersystems. Is it something which needs to be enabled on the server side? Any chance of having this enabled?

haffenloher commented 10 years ago

I assume it is still the case that I cannot send messages to my contacts using the TextSecure client from play store, right? Looking and Release.java the client seems to connect to a websocket URI of whispersystems.

In Release.java, replace the textsecure-service-staging subdomain with textsecure-service. I've been using the websocket client to communicate with normal Textsecure users for a couple of days now and everything works fine. The additional battery drain is noticeable, but that was to be expected.

rehans commented 10 years ago

@brumsoel Thanks a lot! First tests seem to succeed.

JavaJens commented 10 years ago

Sorry I've been absent so long. Glad you succeeded though. Generally speaking I doubt that the feature is ready for serious usage, but any reports are very welcome.

I've updated my branch and hopefully fixed the problems when registering a device without Play services. However I can only test that on the simulator, but first results look promising. Feedback from real devices is welcome!

haffenloher commented 10 years ago

@JavaJens Should I set DISABLE_GCM to false in Release.java prior to testing registration? With the default value of DISABLE_GCM = true, the result seems predictable ;)

JavaJens commented 10 years ago

Yes, set it to false. I will update my branch accordingly ;) But you need to ensure that you don't have Play Services installed.

rehans commented 10 years ago

I debugged the lines in RegistrationActivity.java. The registration process succeeds but the string "GCM not supported. Fallback to WebSocket" is not written to the log. The reason is that isUserRecoverableError returns true and getErrorDialog also returns a valid dialog object. But calling show() does not show the dialog on the device. The next dialog being shown is the AlertDialog (verify number etc.). But it is not harmful to the registration process. The UX could be improved here later ;)

Testing after one and a half day:

  1. I send a message to myself several time in order to test connectivity. It always returns an empty message first. I think this is known already ;)
  2. One time I had a broken "send" icon (bottom right). The blue arrow like icon was replaced by a black box with some colored pixels. Probably has nothing to do with the websockets but I just want to mention.
  3. There is of course the additional delay when the device is sleeping. One message arrived 8 minutes "late" (is fine with me).
  4. The extra battery drain is not noticable on my device. I did not do a detailed measurement though.

I will continue testing of course.

JavaJens commented 10 years ago

@rehans Thanks for debugging. Could it be that the dialog is not shown because no Play services at all are installed? I saw it once on a stock TS and Rom where it complained about Play being outdated.

Nevertheless the idea is that if the error is userRecoverable we will try again later, as it was done already in TS and until successfully registering with GCM use Websockets. The problem is that one cannot distinguish between outdated and not installed Play Also those users who don't want GCM at all can use the DISABLE_GCM flag to prevent the scheduled checks.

@moxie0 I moved the common functions isActiveNumber and initializeTrustManager into Util classes, I hope this fits your code guidelines? Maybe you can also help with these empty messages? On the wire they are all different, thats why I assumed they are encrypted properly. I've attached an exemplary message: {"id":1,"message":"AU9mCbwZw9iH9sinGHPgEftwqihLzspvw5XGRNWKBtZyYCu206PQcHnErAqLMwDVta6Yq+Johb59wws="}

Notably is also that they don't always occur; I sent three messages and got 2 empty ones.

rehans commented 10 years ago

@JavaJens

Could it be that the dialog is not shown because no Play services at all are installed? I saw it once on a stock TS and Rom where it complained about Play being outdated.

Sure, this might be the reason. I've got no Google Play installed on my device. Nevertheless getErrorDialog returns a valid object, this is why I wondered. Probably Null Object Pattern behind.

The problem is that one cannot distinguish between outdated and not installed Play

Ah ok. I thought this was shown by the ConnectionResult being SERVICE_MISSING or SERVICE_VERSION_UPDATE_REQUIRED. In my case it returns SERVICE_MISSING.

Nevertheless the idea is that if the error is userRecoverable we will try again later, as it was done already in TS and until successfully registering with GCM use Websockets.

Ok, so the user will actually not see anything about websockets within the UI. This is good in order to keep it simple.