adorsys / secure-storage-android

Store strings & credentials securely encrypted on your device
Apache License 2.0
367 stars 58 forks source link

NullPointerException on Certificate.getPublicKey #62

Closed dhleong closed 4 years ago

dhleong commented 5 years ago

Using 1.2.2; haven't reproduced this consistently but have a few reports of it in our logs (a bit messy due to retracing):

Caused by SecureStorageException: Attempt to invoke virtual method 'java.security.PublicKey java.security.cert.Certificate.getPublicKey()' on a null object reference
       at de.adorsys.android.securestoragelibrary.KeystoreTool.java.lang.String encryptMessage(android.content.Context,java.lang.String)(KeystoreTool.java:67)
                                                               void generateKeyPair(android.content.Context)
                                                               boolean keyPairExists()
       at de.adorsys.android.securestoragelibrary.SecurePreferences.void removeSecureValue(android.content.Context,java.lang.String)(SecurePreferences.java:13)
                                                                    void setValue(android.content.Context,java.lang.String,java.lang.String)
       at my.app.modules.SecurePrefsEditor.putString(SecurePrefsEditor.java:15)
       at my.app.modules.AuthModule$authUpdater$1$2.call(AuthModule.java:25)
       at my.app.modules.AuthModule$authUpdater$1$2.call(AuthModule.java)
       at io.reactivex.internal.operators.completable.CompletableFromCallable.void subscribeActual(io.reactivex.CompletableObserver)(CompletableFromCallable.java:9)
       at io.reactivex.Completable.io.reactivex.Completable defer(java.util.concurrent.Callable)(Completable.java:14)
                                   io.reactivex.Completable doOnComplete(io.reactivex.functions.Action)
                                   io.reactivex.Completable doOnError(io.reactivex.functions.Consumer)
                                   io.reactivex.Completable doOnLifecycle(io.reactivex.functions.Consumer,io.reactivex.functions.Consumer,io.reactivex.functions.Action,io.reactivex.functions.Action,io.reactivex.functions.Action,io.reactivex.functions.Action)
                                   io.reactivex.Completable observeOn(io.reactivex.Scheduler)
                                   io.reactivex.Completable onErrorComplete(io.reactivex.functions.Predicate)
                                   io.reactivex.disposables.Disposable subscribe(io.reactivex.functions.Action,io.reactivex.functions.Consumer)
                                   void subscribe(io.reactivex.CompletableObserver)
                                   java.lang.NullPointerException toNpe(java.lang.Throwable)
                                   io.reactivex.Single toSingleDefault(java.lang.Object)
       at io.reactivex.internal.operators.completable.CompletableSubscribeOn$SubscribeOnObserver.run(CompletableSubscribeOn.java:2)
       at io.reactivex.internal.schedulers.ScheduledDirectTask.call(ScheduledDirectTask.java:9)
       at io.reactivex.internal.schedulers.ScheduledDirectTask.call(ScheduledDirectTask.java)
       at java.util.concurrent.FutureTask.run(FutureTask.java:266)
       at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
       at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
       at java.lang.Thread.run(Thread.java:764)

Caused by SecureStorageException: Attempt to invoke virtual method 'java.security.PublicKey java.security.cert.Certificate.getPublicKey()' on a null object reference
       at de.adorsys.android.securestoragelibrary.KeystoreTool.java.security.PublicKey getPublicKey(android.content.Context)(KeystoreTool.java:45)
       at de.adorsys.android.securestoragelibrary.KeystoreTool.java.lang.String encryptMessage(android.content.Context,java.lang.String)(KeystoreTool.java:19)
                                                               void generateKeyPair(android.content.Context)
                                                               boolean keyPairExists()
       at de.adorsys.android.securestoragelibrary.SecurePreferences.void removeSecureValue(android.content.Context,java.lang.String)(SecurePreferences.java:13)
                                                                    void setValue(android.content.Context,java.lang.String,java.lang.String)
       at my.app.modules.SecurePrefsEditor.putString(SecurePrefsEditor.java:15)
       at my.app.modules.AuthModule$authUpdater$1$2.call(AuthModule.java:25)
       at my.app.modules.AuthModule$authUpdater$1$2.call(AuthModule.java)
       at io.reactivex.internal.operators.completable.CompletableFromCallable.void subscribeActual(io.reactivex.CompletableObserver)(CompletableFromCallable.java:9)
       at io.reactivex.Completable.io.reactivex.Completable defer(java.util.concurrent.Callable)(Completable.java:14)
                                   io.reactivex.Completable doOnComplete(io.reactivex.functions.Action)
                                   io.reactivex.Completable doOnError(io.reactivex.functions.Consumer)
                                   io.reactivex.Completable doOnLifecycle(io.reactivex.functions.Consumer,io.reactivex.functions.Consumer,io.reactivex.functions.Action,io.reactivex.functions.Action,io.reactivex.functions.Action,io.reactivex.functions.Action)
                                   io.reactivex.Completable observeOn(io.reactivex.Scheduler)
                                   io.reactivex.Completable onErrorComplete(io.reactivex.functions.Predicate)
                                   io.reactivex.disposables.Disposable subscribe(io.reactivex.functions.Action,io.reactivex.functions.Consumer)
                                   void subscribe(io.reactivex.CompletableObserver)
                                   java.lang.NullPointerException toNpe(java.lang.Throwable)
                                   io.reactivex.Single toSingleDefault(java.lang.Object)
       at io.reactivex.internal.operators.completable.CompletableSubscribeOn$SubscribeOnObserver.run(CompletableSubscribeOn.java:2)
       at io.reactivex.internal.schedulers.ScheduledDirectTask.call(ScheduledDirectTask.java:9)
       at io.reactivex.internal.schedulers.ScheduledDirectTask.call(ScheduledDirectTask.java)
       at java.util.concurrent.FutureTask.run(FutureTask.java:266)
       at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
       at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
       at java.lang.Thread.run(Thread.java:764)
luckyhandler commented 5 years ago

hey @dhleong, so are you sure that this bug only appears in the newest version? Would make finding the bug easier for us...

dhleong commented 5 years ago

I'm not certain, but it's possible. I was using 1.1.1 previously and I don't see any reports of this issue in my logs for the old version that used that at a cursory glance.

dhleong commented 5 years ago

Unfortunately we haven't been able to reproduce this locally, but given the stacktrace here it seems likely this is caused by the changes in https://github.com/adorsys/secure-storage-android/commit/320eb4544a1d4c4b2fa4c282889485de7fd40532. Do you happen to know the context behind that change? The associated PR just says "fixes the keyPairExists function" which isn't very illuminating.

For what it's worth, we have one of the devices that supposedly reproduces it, but it's running 6.0.1; the reports seem to be mostly on Android 8 and 9 (indeed, the Moto Z Play report is from 8.0).

I thought perhaps the version switches were causing an issue if a user upgraded across the boundary, but we have at least one user saying they reinstalled the app and still encounter the issue.

Please let me know if there's anything else I can do to move this along!

luckyhandler commented 5 years ago

@drilonrecica any idea?

kibotu commented 5 years ago

make sure that your context is valid @dhleong

e.g. use application context or service context to interact with secure storage, instead of e.g. activity

drilonrecica commented 5 years ago

@dhleong @luckyhandler I will look into this, thanks for bringing it to our attention.

dhleong commented 5 years ago

@kibotu Yep, I wrote a wrapper class around this library to emulate a regular SharedPreferences API that has the Application context injected via Dagger. It's definitely not an Activity context.

@drilonrecica Thanks!

luckyhandler commented 5 years ago

@kibotu the context shouldn't be a problem as we internally use the application context whatever you pass.

drilonrecica commented 4 years ago

Version 1.2.4 is out now. The bug should not reappear. I have tested on ,multiple devices and emulator with different API levels and different installation configurations and cannot reproduce the bug with the logic of v.1.2.4

Thank you all for the contributions to SecureStorage

Lovrenc commented 4 years ago

Since the bug caused the library to find a certificate but it had no public key, will updating to 1.2.4 fix the issue for the users already affected by it?