airgap-it / beacon-android-sdk

The beacon sdk allows Android developers of dApps and wallets on Tezos to implement the wallet interaction standard tzip-10.
https://walletbeacon.io
MIT License
10 stars 8 forks source link

Issues with secure shared preferences #15

Open AniketSindhu opened 1 year ago

AniketSindhu commented 1 year ago

Bug Report

Current Behavior

For some of the users beacon android sdk is throwing this error

 com.google.crypto.tink.shaded.protobuf.InvalidProtocolBufferException: Protocol message contained an invalid tag (zero).
     at com.google.crypto.tink.shaded.protobuf.ArrayDecoders.decodeUnknownField(ArrayDecoders.java:1036)
     at com.google.crypto.tink.shaded.protobuf.MessageSchema.parseProto3Message(MessageSchema.java:5426)
     at com.google.crypto.tink.shaded.protobuf.MessageSchema.mergeFrom(MessageSchema.java:5442)
     at com.google.crypto.tink.shaded.protobuf.ArrayDecoders.decodeMessageField(ArrayDecoders.java:246)
     at com.google.crypto.tink.shaded.protobuf.ArrayDecoders.decodeMessageList(ArrayDecoders.java:704)
     at com.google.crypto.tink.shaded.protobuf.MessageSchema.parseProto3Message(MessageSchema.java:5373)
     at com.google.crypto.tink.shaded.protobuf.MessageSchema.mergeFrom(MessageSchema.java:5442)
     at com.google.crypto.tink.shaded.protobuf.GeneratedMessageLite.parsePartialFrom(GeneratedMessageLite.java:1567)
     at com.google.crypto.tink.shaded.protobuf.GeneratedMessageLite.parseFrom(GeneratedMessageLite.java:1680)
     at com.google.crypto.tink.proto.Keyset.parseFrom(Keyset.java:958)
     at com.google.crypto.tink.integration.android.SharedPrefKeysetReader.read(SharedPrefKeysetReader.java:84)
     at com.google.crypto.tink.CleartextKeysetHandle.read(CleartextKeysetHandle.java:61)
     at com.google.crypto.tink.integration.android.AndroidKeysetManager$Builder.read(AndroidKeysetManager.java:332)
     at com.google.crypto.tink.integration.android.AndroidKeysetManager$Builder.readOrGenerateNewKeyset(AndroidKeysetManager.java:288)
     at com.google.crypto.tink.integration.android.AndroidKeysetManager$Builder.build(AndroidKeysetManager.java:239)
     at androidx.security.crypto.EncryptedFile$Builder.build(EncryptedFile.java:233)
     at it.airgap.beaconsdk.core.internal.storage.sharedpreferences.encryptedfile.TargetEncryptedFileManager$write$2$1.invokeSuspend(TargetEncryptedFileManager.kt:49)
     at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
     at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106)
     at kotlinx.coroutines.internal.LimitedDispatcher.run(LimitedDispatcher.kt:42)
     at kotlinx.coroutines.scheduling.TaskImpl.run(Tasks.kt:95)
     at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:570)
     at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:749)
     at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:677)
     at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:664)

How to Reproduce?

We are also using secured shared preference in our app. I think it might be due to some conflicts

Environment

Additional Context

jsamol commented 1 year ago

Hi! Thanks for the report, I did an initial investigation and for now it looks to me like it's related to an EncryptedSharedPreferences issue that some have been reporting for the androidx.security library.

The only difference is that we don't actually use EncryptedSharedPreference in Beacon, instead we use EncryptedFile, but looking at the stack traces people posted there, the initialization processes of both end up invoking the same portion of code eventually leading to the error. I'll have to investigate further, but for now I can't see any fix or workaround which has been more or less confirmed and which we could apply in Beacon. A library shouldn't exclude files from being backed up nor completely delete shared preferences it hasn't created internally, I believe such steps should be taken only in the end product to avoid weird and unexpected behaviour.

What I'm slightly concerned about is that you mentioned you also use parts of the security library in your application so there's a chance your implementation and ours in Beacon interfere with each other causing the problem. If it turns up that your implementation just doesn't work alongside Beacon's (or vice versa), I'd be happy to find a way to fix it in the library, but then I'll need an MVP (it could be your app, if it's open source, with an instruction on how to build and run it) and steps to reproduce it with a list of known affected devices (I need a list to increase my chances of getting my hands on such a device 😅), since it looks like it's not happening on every device out there.

AniketSindhu commented 1 year ago

Yes, Our source code is open source and available at https://github.com/Tezsure/naan-app. We are still trying to figure out how to reproduce this issue since it only occurs on some devices in certain cases. Currently, all the reports we have received are from users whenever they try to update the app and open it. Clearing the cache resolves the problem.

hawkbee1 commented 1 year ago

Hi Looks like we have same issue with Altme wallet: https://github.com/TalaoDAO/AltMe I'm having issue on my Pixel 6 so I may be able to help. When deleting storage after crashing the apps works fine (just clearing cache doesn't work)

Phone having this issue also have issue with flutter package called flutter_secure_storage: https://github.com/mogol/flutter_secure_storage/issues/354 I'm able to solve flutter secure storage issue by upgrading minSdkVersion to 23 in our gradle and using encryptedSharedPreferences with a sharedPreferencesName.

E/AndroidRuntime(31923): FATAL EXCEPTION: DefaultDispatcher-worker-3
E/AndroidRuntime(31923): Process: co.altme.alt.me.altme, PID: 31923
E/AndroidRuntime(31923): com.google.crypto.tink.shaded.protobuf.c0: Protocol message contained an invalid tag (zero).
E/AndroidRuntime(31923):    at com.google.crypto.tink.shaded.protobuf.e.G(ArrayDecoders.java:4)
E/AndroidRuntime(31923):    at com.google.crypto.tink.shaded.protobuf.v0.d0(MessageSchema.java:56)
E/AndroidRuntime(31923):    at com.google.crypto.tink.shaded.protobuf.v0.b(MessageSchema.java:2)
E/AndroidRuntime(31923):    at com.google.crypto.tink.shaded.protobuf.e.p(ArrayDecoders.java:5)
E/AndroidRuntime(31923):    at com.google.crypto.tink.shaded.protobuf.e.q(ArrayDecoders.java:1)
E/AndroidRuntime(31923):    at com.google.crypto.tink.shaded.protobuf.v0.d0(MessageSchema.java:51)
E/AndroidRuntime(31923):    at com.google.crypto.tink.shaded.protobuf.v0.b(MessageSchema.java:2)
E/AndroidRuntime(31923):    at com.google.crypto.tink.shaded.protobuf.z.J(GeneratedMessageLite.java:3)
E/AndroidRuntime(31923):    at com.google.crypto.tink.shaded.protobuf.z.G(GeneratedMessageLite.java:2)
E/AndroidRuntime(31923):    at t3.i0.W(Unknown Source:2)
E/AndroidRuntime(31923):    at n3.d.read(Unknown Source:8)
E/AndroidRuntime(31923):    at g3.c.a(Unknown Source:0)
E/AndroidRuntime(31923):    at n3.a$b.e(AndroidKeysetManager.java:4)
E/AndroidRuntime(31923):    at n3.a$b.f(AndroidKeysetManager.java:1)
E/AndroidRuntime(31923):    at n3.a$b.d(AndroidKeysetManager.java:3)
E/AndroidRuntime(31923):    at androidx.security.crypto.a$a.a(EncryptedFile.java:7)
E/AndroidRuntime(31923):    at it.airgap.beaconsdk.core.internal.storage.sharedpreferences.encryptedfile.TargetEncryptedFileManager$write$2$1.invokeSuspend(TargetEncryptedFileManager.kt:8)
E/AndroidRuntime(31923):    at kotlin.coroutines.jvm.internal.a.resumeWith(ContinuationImpl.kt:4)
E/AndroidRuntime(31923):    at kotlinx.coroutines.d1.run(DispatchedTask.kt:18)
E/AndroidRuntime(31923):    at kotlinx.coroutines.internal.m.run(LimitedDispatcher.kt:2)
E/AndroidRuntime(31923):    at kotlinx.coroutines.scheduling.k.run(Tasks.kt:1)
E/AndroidRuntime(31923):    at kotlinx.coroutines.scheduling.a.r(CoroutineScheduler.kt:1)
E/AndroidRuntime(31923):    at kotlinx.coroutines.scheduling.a$c.d(CoroutineScheduler.kt:4)
E/AndroidRuntime(31923):    at kotlinx.coroutines.scheduling.a$c.n(CoroutineScheduler.kt:4)
E/AndroidRuntime(31923):    at kotlinx.coroutines.scheduling.a$c.run(Unknown Source:0)
E/AndroidRuntime(31923):    Suppressed: kotlinx.coroutines.b1: [v2{Cancelling}@25b2f9b, Dispatchers.IO]

@AniketSindhu you're also using https://github.com/TalaoDAO/beacon ? @bibash28