redfish64 / TinyTravelTracker

Private Android GPS tracker
GNU General Public License v3.0
135 stars 31 forks source link

Password issue #80

Open klauf opened 6 years ago

klauf commented 6 years ago

Hi all Thanks for this great piece of software. Wanted to give it a try, installed it on my Huawei p20 (8.1),followed the assistent steps successfully, and then it asks for a password to finally open the app, no matter if I choose 'no password' during the assistant, or if I define one, I'm stuck with 'password incorrect'. Any idea what it might be? best regards

benzntech commented 6 years ago

I too had the same issue. Not able to login with the password i provide. Seems to be serious bug

redfish64 commented 6 years ago

It might be because of Android 8.1 being incompatible somehow. I'm downloading an Android 8.1 emulator and will get back to you.

@bensonkb Can you confirm (or not) that you are also using a phone with Android 8.1?

redfish64 commented 6 years ago

I tried it out on an emulator of a Pixel 2 running android 8.1 and it worked fine. I'll try and think of what the next step can be.

If anyone else has this problem, please let me know which phone and which version of android (Under Settings/About Phone/Android version)

benzntech commented 6 years ago

@redfish64 👍 :- I have attached a Video for the same along with this link. Issue

Rudertier commented 6 years ago

Hey. I can confirm the issue. On a new Nokia 6.1 with android 8.1.0. Also I am not setting any password at all.

redfish64 commented 6 years ago

I can't find an emulator to simulate this problem, so I decided to try another route.

The code that verifies the password and decrypts the private key that TTT uses to record points catches all exceptions and assumes that the user entered a bad password. I did this because I was getting different exceptions depending on what was entered, and figured that different phones might also have different errors, but meaning the same thing.

Anyway, I created a new version, 1.1.40, which should be up in a couple of days. If the code fails to decrypt the password 3 times in a row, it will assume there is another issue and throw an exception, which will be caught by my remote logging system.

So, if you download the new version, and enter in your cause the problem to occur 3 times in a row, you will trigger this. I will then be able to at least see what the underlying exception is, and be able to move forward.

Also, if you know how to use "adb logcat" v 1.1.40 logs the issue to the android log. Just search for "GpsTrailer: Password decryption exception" in the log file, and the exception will appear right after it.

BTW, if you specify no password, the code uses a default password that it automatically supplies to itself. I do this because the data is held on the sdcard, and any other app with external storage can read/write any data on the sdcard, which is a security issue. So this way, others apps can't read TTT data points.

Rudertier commented 6 years ago

Hi, I would like to ask for an update on this. Did you receive any logs? Is activating the sending of logs and reproducing the issue enough? Thanks a lot

eebssk1 commented 6 years ago

same problem here,no password is set. Android 8.1 crash report caught by xposed based crash handle module

java.lang.IllegalStateException: Could not execute method for android:onClick at android.view.View$DeclaredOnClickListener.onClick(View.java:5394) at android.view.View.performClick(View.java:6314) at android.view.View$PerformClick.run(View.java:24790) at android.os.Handler.handleCallback(Handler.java:790) at android.os.Handler.dispatchMessage(Handler.java:99) at android.os.Looper.loop(Looper.java:164) at android.app.ActivityThread.main(ActivityThread.java:6499) at java.lang.reflect.Method.invoke(Native Method) at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:440) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:807) at de.robv.android.xposed.XposedBridge.main(XposedBridge.java:108) Caused by: java.lang.reflect.InvocationTargetException at java.lang.reflect.Method.invoke(Native Method) at android.view.View$DeclaredOnClickListener.onClick(View.java:5389) ... 10 more Caused by: java.lang.IllegalStateException: HACK!!! Assuming if the user entered the wrong password 3 times then there is something else that is wrong!!! at com.rareventure.gps2.GpsTrailerCrypt.decryptPrivateKey(GpsTrailerCrypt.java:249) at com.rareventure.gps2.GpsTrailerCrypt.initializePrivateKey(GpsTrailerCrypt.java:190) at com.rareventure.gps2.GpsTrailerCrypt.initialize(GpsTrailerCrypt.java:150) at com.rareventure.gps2.GTG.requireEncryptAndDecrypt(GTG.java:557) at com.rareventure.gps2.reviewer.password.EnterPasswordActivity.onOk(EnterPasswordActivity.java:96) ... 12 more Caused by: java.security.spec.InvalidKeySpecException: com.android.org.conscrypt.OpenSSLX509CertificateFactory$ParsingException: Error parsing private key at com.android.org.conscrypt.OpenSSLKey.getPrivateKey(OpenSSLKey.java:305) at com.android.org.conscrypt.OpenSSLRSAKeyFactory.engineGeneratePrivate(OpenSSLRSAKeyFactory.java:71) at java.security.KeyFactory.generatePrivate(KeyFactory.java:395) at com.rareventure.gps2.GpsTrailerCrypt.decryptPrivateKey(GpsTrailerCrypt.java:235) ... 16 more Caused by: com.android.org.conscrypt.OpenSSLX509CertificateFactory$ParsingException: Error parsing private key at com.android.org.conscrypt.NativeCrypto.EVP_parse_private_key(Native Method) at com.android.org.conscrypt.OpenSSLKey.getPrivateKey(OpenSSLKey.java:303) ... 19 more

Rudertier commented 5 years ago

Hi, I don't mean to be annoying, but any progress? I'd like to stop carrying my old, broken phone around just for tracking. ;-)

eebssk1 commented 5 years ago

Well,,the problem gone away after i flashed my own custom rom build...

MarvinMep commented 5 years ago

Hi, everything used to work fine for me until yesterday. I never set a password, I already used 1.1.41 TTT version, but Android was upgraded yesterday from 7.x to 8.1, and since then TTT asks for a password.

redfish64 commented 5 years ago

I can't reproduce this because I don't have an 8.1 phone, and the 8.1 emulator doesn't have this problem. My guess is that the symmetric encryption algorithm has an issue on some 8.1 phones. I could try switching to another one, but I want to confirm that's the case.

If you want to, I created a simple test, https://github.com/redfish64/MrCrypto with a compiled signed apk here:

https://github.com/redfish64/MrCrypto/releases/download/untagged-9da75a2f5166c5fc9136/app-release.apk

Run it, take a screenshot and put it here, or email it to me at engler@gmail.com

klauf commented 5 years ago

Hi Tim

The apk link is broken 404, can you please check?

Br Laurent

On 20 October 2018 04:08:36 CEST, Tim Engler notifications@github.com wrote:

I can't reproduce this because I don't have an 8.1 phone, and the 8.1 emulator doesn't have this problem. My guess is that the symmetric encryption algorithm has an issue on some 8.1 phones. I could try switching to another one, but I want to confirm that's the case.

If you want to, I created a simple test, https://github.com/redfish64/MrCrypto with a compiled signed apk here:

https://github.com/redfish64/MrCrypto/releases/download/untagged-9da75a2f5166c5fc9136/app-release.apk

Run it, take a screenshot and put it here, or email it to me at engler@gmail.com

-- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/redfish64/TinyTravelTracker/issues/80#issuecomment-431540448

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

h2mswb6h commented 5 years ago

I have Tracked down the issue. The buffer being supplied to the keyfactory in GpsTrailerCrypt.java on line 232. PKCS8EncodedKeySpec keySpec = new PKCS8EncodedKeySpec(output);

In android 8.1 the encryption library was updated. It will raise an "InvalidKeySpecException" when the supplied buffer is longer than the key when using the RSA key factory. This exception caught on line 236. In the comment states that //we are assuming all errors are because of a wrong password Thus it believes that the user has enter the wrong password. Or in the case of no password the default was wrong, and then it will ask the user for a password.

The fix is to use the correct buffer size. This correctly sized buffer is already calculated. It is named output2. This is seen in the code block below.

`
Crypt keyDecryptor = new Crypt(Crypt.getRawKey(password, prefs.salt)); byte[] encryptedPrivateKey = prefs.encryptedPrivateKey;

        byte[] output = new byte[keyDecryptor
                .getNumOutputBytesForDecryption(encryptedPrivateKey.length)];

        int keyLength = keyDecryptor.decryptData(output,
                encryptedPrivateKey);

        byte[] output2 = new byte[keyLength];
        System.arraycopy(output, 0, output2, 0, keyLength);

        // Private keys are encoded with PKCS#8 (or so they say)
        PKCS8EncodedKeySpec keySpec = new PKCS8EncodedKeySpec(output);

        KeyFactory keyFactory = KeyFactory.getInstance("RSA");
                    return keyFactory.generatePrivate(keySpec);

`

The output 2 is the right length and copy is used to fill that buffer. But on the affecting line output is used where output2 should be used. This solves the issue on my own phone and should on others.

As for some custom roms and emulators working some builds do not always include the latest version of the crypto libraries, depending on the hardware. I have been able to reproduce this on the emulator in android studio on Linux 64bit.

I fixed this in early jun before this issue was opened on my own phone. I assumed that it would be fixed before the next update, but it looks like it was not. I made an github account just to fix summit this. Sorry for keeping this from others, just thought it was a simple fix to find(less then 3 hours, starting from no android or java experience) that it would be found and fixed by developers. Now that I need to update the app, and it not being fixed I found this issue and summit this.

redfish64 commented 5 years ago

Thanks. I'll try and get this in today.

On Sat, Oct 27, 2018, 08:26 h2mswb6h notifications@github.com wrote:

I have Tracked down the issue. The buffer being supplied to the keyfactory in GpsTrailerCrypt.java on line 232. PKCS8EncodedKeySpec keySpec = new PKCS8EncodedKeySpec(output);

In android 8.1 the encryption library was updated. It will raise an "InvalidKeySpecException" when the supplied buffer is longer than the key when using the RSA key factory. This exception caught on line 236. In the comment states that //we are assuming all errors are because of a wrong password Thus it believes that the user has enter the wrong password. Or in the case of no password the default was wrong, and then it will ask the user for a password.

The fix is to use the correct buffer size. This correctly sized buffer is already calculated. It is named output2. This is seen in the code block below.

` Crypt keyDecryptor = new Crypt(Crypt.getRawKey(password, prefs.salt)); byte[] encryptedPrivateKey = prefs.encryptedPrivateKey;

  byte[] output = new byte[keyDecryptor
          .getNumOutputBytesForDecryption(encryptedPrivateKey.length)];

  int keyLength = keyDecryptor.decryptData(output,
          encryptedPrivateKey);

  byte[] output2 = new byte[keyLength];
  System.arraycopy(output, 0, output2, 0, keyLength);

  // Private keys are encoded with PKCS#8 (or so they say)
  PKCS8EncodedKeySpec keySpec = new PKCS8EncodedKeySpec(output);

  KeyFactory keyFactory = KeyFactory.getInstance("RSA");
                return keyFactory.generatePrivate(keySpec);

`

The output 2 is the right length and copy is used to fill that buffer. But on the affecting line output is used where output2 should be used. This solves the issue on my own phone and should on others.

As for some custom roms and emulators working some builds do not always include the latest version of the crypto libraries, depending on the hardware. I have been able to reproduce this on the emulator in android studio on Linux 64bit.

I fixed this in early jun before this issue was opened on my own phone. I assumed that it would be fixed before the next update, but it looks like it was not. I made an github account just to fix summit this. Sorry for keeping this from others, just thought it was a simple fix to find(less then 3 hours, starting from no android or java experience) that it would be found and fixed by developers. Now that I need to update the app, and it not being fixed I found this issue and summit this.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/redfish64/TinyTravelTracker/issues/80#issuecomment-433575453, or mute the thread https://github.com/notifications/unsubscribe-auth/AB1W0rHhn1jayrHFFs12QcfJ6ueU6hmnks5uo6gzgaJpZM4U47bF .

Rudertier commented 5 years ago

Hey! It works! Thanks a lot.

MarvinMep commented 5 years ago

Hi! Now it works! Thank you!! I have only one more comment/suggestion: If you think password is strict necessary, then force the user defining one (and provide a good password recovery system). This approaching with a default password (which users are unawere of) is risky (the security system ends up protecting data from its own owner). Either you can also simply be more strongly to warn users about the risks of not set a password (and leave the data open in sdcard).

redfish64 commented 5 years ago

The problem is the data is on the sdcard which makes it public so any other app with sdcard access can read it. Without encryption any other app could steal your points. The default password just makes the code easier.

You have access to your points through backup/restore to GPX.

Márcio Vinícius Pinheiro notifications@github.com 於 2018年11月2日 週五 23:00寫道:

Hi! Now it works! Thank you!! I have only one more comment/suggestion: If you think password is strict necessary, then force the user defining one (and provide a good password recovery system). This approaching with a default password (which users are unawere of) is risky (the security system ends up protecting data from its own owner). Either you can also simply be more strongly to warn users about the risks of not set a password (and leave the data open in sdcard).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/redfish64/TinyTravelTracker/issues/80#issuecomment-435408435, or mute the thread https://github.com/notifications/unsubscribe-auth/AB1W0lR_12D54tRBeWJJMmOEs3dQKKCUks5urF4KgaJpZM4U47bF .