gpfjeff / android-ppp Goto Github PK

After a careful review of our code, I have determined that PPP for Android is 
technically susceptible to the recently discovered Java Cryptography 
Architecture flaw in Android:

http://android-developers.blogspot.com/2013/08/some-securerandom-thoughts.html

PPP uses java.security.SecureRandom to generate random 256-bit sequence keys 
whenever a new card set is created. Based on the information provided by 
Google, these sequence keys will *NOT* be cryptographically strong and *MAY* be 
repeatable and therefore insecure. If, however, you intend to use a sequence 
key generated from another source (i.e., you are given a sequence key by the 
service you are planning to use PPP to authenticate with), those sequence keys 
should be acceptable (provided the source has a suitably strong PRNG). In 
short, until I can work on a fix for this issue, it is strongly recommended 
that you *DO NOT USE* sequence keys automatically generated by PPP for Android 
for authentication. Use sequence keys generated from another source until we 
can push out a suitable fix.

I recently upgraded my personal phone to a Samsung Galaxy S4 and ran into a 
blocking issue regarding the text size in the card view. As displayed in the 
first attached screenshot, using the default card settings (ten rows per card, 
seven columns per card, four-character passcodes) renders the text size so 
large that the last two columns become unreadable in portrait format. Because 
of a number of limitations within Android itself, the user is unable to scroll 
horizontally to see the missing two columns. All seven columns are displayed in 
landscape orientation, so the app is technically still usable. However, the 
text should not be rendered so large on such a small screen.

Fortunately, I know *why* this is happening and fixing it should be relatively 
simple, although it may require some experimentation. It has to do with the 
detected screen resolution.

Earlier versions of Android broke screen resolutions down into several general 
classes: low (ldpi), medium (mdpi, the default), high (hdpi), and extra high 
(xhdpi). Both my old Motorola Droid 3 and my Samsung Galaxy Note 10.1 count as 
medium-size screens based on their resolutions, rather than physical 
dimensions. The S4 is detected as extra-high, with a screen density > 400dpi. 
Thus, PPP is likely trying to use a tablet-optimized larger text size to 
accommodate the higher resolution, while the smaller screen actually cuts 
things off.

Later versions of Android actually provide mechanisms to detect and target 
screen sizes versus resolution, but to maintain backward compatibility, we 
can't take advantage of them.

I had to make a few tweaks to get the text size larger on the tablet since it 
had the same resolution values as the old phone, so I probably just need to 
make the same corrections here. It will take some trial and error, and I have 
no idea how long it's going to take for me to get around to fixing this. 
Unfortunately, that will probably mean that some folks with newer devices that 
have extra high resolution screens might find PPP unusable until I can push out 
a fix.

See the wiki article "FutureFeatures" for the full details:

https://code.google.com/p/android-ppp/wiki/FutureFeatures

PPP should allow the user to backup card set parameters and, optionally, 
strike-out data to an encrypted file on the device's internal or external 
storage (i.e. SD card). Conversely, the user should also be able to restore 
backups by importing an previously exported file back into PPP. Card set data 
could also be copied from one device to another via this mechanism, such as for 
users who have multiple devices or from someone replacing one device with 
another.

If PPP is locked with a password, the user must enter the correct password at 
the prompt to unlock PPP and access their card list. Currently, typing the 
Enter key, either on a physical or soft keyboard, does nothing; the user must 
tap the Unlock button to unlock the app. PPP should accept tapping the Enter 
key as the same as tapping the Unlock button, which will make entering 
passwords easier on devices with physical keyboards.

This *should* be a simple fix. I have working code from the Crpytnos app that 
does something similar (tapping Enter generates the user's pseudo-random 
password). I tried porting that code over to PPP and it works... sort of. The 
app unlocks, but the Back button behavior is not ideal. If the user taps Back 
to return to the card set list, tapping Back again should return the user to 
the previously used app (or the Home screen if no previous app was in use). 
This is the expected functionality and what currently occurs if the user taps 
the Unlock button on the screen. However, if the Enter key is tapped, tapping 
back from the card set list appears to simply redisplay the card set list, 
forcing the user to tap Back twice to actually exit.

This is an artifact of how the password screen is implemented. The password 
screen is itself a separate "activity" in Android. If the user has no password 
set, the password activity simply passes control to the card set list activity 
and removes itself from the activity call stack. If a password has been set, it 
waits for the user to enter the correct password before passing control to the 
card set list. However, it is still supposed to remove itself from the call 
stack, so hitting Back from the card set list screen should exit the app. 
Apparently, the Cryptnos approach for some reason does not remove the password 
activity from the stack. Hitting back from the card set list screen tries to 
return the user to the password screen, which dutifully says the password was 
valid returns control back to the card set screen. Then and only then does it 
remove itself from the call stack and hitting Back actually exit the app. This 
isn't a problem in Cryptnos because that app isn't launching another activity; 
it's just triggering a button's event code to achieve the same functionality.

I *think* I know why it's doing this. The test for the Enter key occurs in a 
key listener on the password text box, which looks for the Enter key code. When 
it finds it, it calls the Unlock button event code, essentially "clicking" the 
Unlock button. That event method launches the card set list activity, then 
waits for the user to return. In the case of actually tapping Unlock, returning 
ends the event method and exits the app. But in the Enter case, the Unlock 
event method ends and returns control back to the password box key listener. 
Somehow this ends up bouncing us back to the card set list.

To fix this, I think both the Unlock click listener and the password box key 
listener need to call a single private method that verifies the password and 
either sends the user to the card set list activity or displays the wrong 
password error "toast". Then each event method is handled atomically, without 
one calling the other. There's a possibility, however, that this kind of nested 
call might cause the same problem. In a worst-case scenario, I could copy the 
actual logic from the Unlock event code to the key listener, but I'd rather 
reuse code in a private method if I can so that logic only has to be updated in 
one place.

This has been an annoyance for me for a while and I thought the Cryptnos code 
would fix it. In this case, I think it points to the right direction, but I'll 
have to tailor it for PPP's specific need.

I don't have an ETA for this. I actually tried to put this in before releasing 
the app to Google Play, but had to back it out. (That's why the Enter trick 
still does not work in the official release.)

See the wiki article "FutureFeatures" for full details:

https://code.google.com/p/android-ppp/wiki/FutureFeatures

If sites implementing PPP could generate a QR code containing the card set 
parameters in some standard way, we could use the device's camera to import 
those parameters quickly. This could be implemented in a number of ways. For 
example, Crypnos uses third-party QR code reading apps to actually do the work; 
we could also directly use ZXing library to do it ourselves. A standard format 
will need to be established, however, and it would be ideal if others 
implemented it as well (assuming there *are* other apps out there).

Another possibility is the import and export of parameters via QR code, similar 
to what Cryptnos does, to copy card set parameters between devices. This would 
be a lesser priority, however.