kryptco / krypton-android

DEPRECATED Krypton turns your Android device into a U2F Authenticator: strong, unphishable 2FA.
https://krypt.co
Other
203 stars 50 forks source link

PairScanner bound to MainActivity lifecycle #56

Closed kennylevinsen closed 7 years ago

kennylevinsen commented 7 years ago

Problem

The QR code scanner runs whenever the main activity is active. This consumes excessive battery, and can lead to pairing prompts without the user asking for pairing.

Expected behavior

The QR code scanner runs only when the pairing screen is made active by the user.

Cause

PairScanner (the QR code scanner) is managed by onPause/onResume on PairFragment. PairFragment is tied through the SectionsPagerAdapter to MainActivity. As fragment lifecycles follow their parent activities, this means that PairScanner ends up managed by MainActivity's onPause/onResume, and PairScanner therefore runs whenever MainActivity is active.

Being the primary UI, this basically means that the QR scanner runs whenever Kryptonite is open.

Solution

I suspect the only proper way to do this would be to manually notify the fragments about their visibility from MainActivity on tab switch. This shouldn't be that hard to do.

kcking commented 7 years ago

Thanks for the analysis! The reason for this behavior was because the UI lags quite a bit when the camera is activated or deactivated. As a happy medium, the camera is now released if the pair fragment has not been selected for more than 10s (or immediately on onPause).

kennylevinsen commented 7 years ago

What devices do you see this lag on? If it happens on "fast" devices as well, I might have a look at it - the other apps on my device that have camera views do not see any lag when the view enables. I am unfortunately having some difficulty setting up kryptonite-android on my mac due to ssh-wire's ring dependency.

I have found some things that could be to blame, such as coarse-grained locking on the UI thread (there shouldn't be any locking on the UI thread). If I manage to get the toolchain up and running, I might start combing that out.

kcking commented 7 years ago

The lag happened on a Nexus 5 and less so but still noticeably on a Pixel. The delayed stopping has seemed to fix it though.

Does ring fail at link stage? I'd be happy to help you set up the toolchain. If you are only building for android, you can comment out the cargo lipo step of build.sh

kennylevinsen commented 7 years ago

I finally managed to get it to build - libsodium-jni (dependencies, build scripts I had to modify, and apparently a dead submodule object in 1.0.3, so I had to use a later version) and ring (android NDK fun) took a while to get right. This would have been so much easier if you used pure rust components. :)

I can see the freeze - the animation "jerks" when going to camera view. I'll start some simplifying cleanup in that area, see where that gets me. I should at least be able to remove all the locks using UI thread synchronization, or using just atomic variables (access to a boolean - as well as basically anything but long/double - is guaranteed to be atomic in Java, so all you need is a "volatile" keyword to guarantee memory visibility - no locks needed).

kennylevinsen commented 7 years ago

Fixed by #59.