okta / okta-oidc-android

OIDC SDK for Android
https://github.com/okta/okta-oidc-android
Other
60 stars 45 forks source link

Intermittent encryption error caused by BaseEncryptionManager #223

Closed dragos-okta closed 2 years ago

dragos-okta commented 3 years ago

When using the following SDK versions

implementation "com.okta.authn.sdk:okta-authn-sdk-api:2.0.0" runtimeOnly "com.okta.authn.sdk:okta-authn-sdk-impl:2.0.0" implementation "com.okta.android:oidc-androidx:1.0.17" runtimeOnly "com.okta.sdk:okta-sdk-okhttp:2.0.0"

approximately 350 users out of 4000 are experiencing an intermittent issue from BaseEncryptionManager, mostly on Samsung Galaxy S10 on Android 10, generating the following stack trace

_Caused by com.okta.oidc.util.AuthorizationException Attempt to invoke virtual method 'java.security.PublicKey java.security.cert.Certificate.getPublicKey()' on a null object reference com.okta.oidc.clients.SyncAuthClientImpl.signIn (SyncAuthClientImpl.java:115) com.okta.oidc.clients.AuthClientImpl.lambda$signIn$2 (AuthClientImpl.java:69) com.okta.oidc.clients.-$$Lambda$AuthClientImpl$bd2LRazdR1NgrGeRE1VrsI8D_uI.run (-.java:8) java.util.concurrent.Executors$RunnableAdapter.call (Executors.java:462) java.util.concurrent.FutureTask.run (FutureTask.java:266) java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1167) java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:641) java.lang.Thread.run (Thread.java:919) Caused by java.lang.NullPointerException Attempt to invoke virtual method 'java.security.PublicKey java.security.cert.Certificate.getPublicKey()' on a null object reference com.okta.oidc.storage.security.BaseEncryptionManager.initEncodeCipher (BaseEncryptionManager.java:225) com.okta.oidc.storage.security.BaseEncryptionManager.initCipher (BaseEncryptionManager.java:188) com.okta.oidc.storage.security.BaseEncryptionManager.encrypt (BaseEncryptionManager.java:252) com.okta.oidc.storage.security.DefaultEncryptionManager.encrypt (DefaultEncryptionManager.java:46) com.okta.oidc.storage.OktaRepository.getEncrypted (OktaRepository.java:201) com.okta.oidc.storage.OktaRepository.save (OktaRepository.java:81) com.okta.oidc.OktaState.save (OktaState.java:70) com.okta.oidc.clients.AuthAPI.obtainNewConfiguration (AuthAPI.java:106) com.okta.oidc.clients.SyncAuthClientImpl.signIn (SyncAuthClientImpl.java:80) com.okta.oidc.clients.AuthClientImpl.lambda$signIn$2 (AuthClientImpl.java:69) com.okta.oidc.clients.-$$Lambda$AuthClientImpl$bd2LRazdR1NgrGeRE1VrsI8DuI.run (-.java:8) java.util.concurrent.Executors$RunnableAdapter.call (Executors.java:462) java.util.concurrent.FutureTask.run (FutureTask.java:266) java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1167) java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:641) java.lang.Thread.run (Thread.java:919)

Sample code used

_override fun login(email: String, password: String): Single { return Single.create { subscriber -> Timber.i("Okta auth service login for ${email.hashCode()}")

authenticationClient.authenticate(email, password.toCharArray(), "", null)?.run { val user = this.user val signInCallback = object : RequestCallback<Result, AuthorizationException> { override fun onSuccess(result: Result) { Timber.i("Okta login successful for ${email.hashCode()}") lfsSharedPreferences.setLoginId(email) saveTokensToLFSSharedPrefs(authClient.sessionClient.tokens) lfsSharedPreferences.setName(user.firstName) subscriber.onSuccess(AuthenticationLoginResult.Complete()) }

override fun onError(error: String?, ex: AuthorizationException?) { Timber.e(ex, "Okta login onError for ${email.hashCode()}") subscriber.tryOnError(ex ?: AuthenticationError.NotAuthorized()) } }

Timber.i("Okta login signIn sessionToken isNullOrBlank: ${sessionToken.isNullOrBlank()} for ${email.hashCode()}") authClient.signIn(sessionToken, null, signInCallback) } } .onErrorResumeNext { Timber.e(it, "Okta login onErrorResumeNext for ${email.hashCode()}") Single.error(mapException(it)) } .subscribeOn(Schedulers.io()) }_

Raising this issue as discussed with @FeiChen-okta

JayNewstrom commented 3 years ago

I created an internal issue for this: OKTA-380935.

If you could get steps to reproduce this it would make it a lot easier for us to triage! But either way we will take a look.

FeiChen-okta commented 3 years ago

I created an internal issue for this: OKTA-380935.

If you could get steps to reproduce this it would make it a lot easier for us to triage! But either way we will take a look.

@JayNewstrom @NikitaAvraimov-okta I've added some comments in the ticket.

TheFuzzard commented 3 years ago

@FeiChen-okta @JayNewstrom it seems that version 1.0.18 might have resolved the previous problem, but we still see a NPE during the signIn call

Caused by com.okta.oidc.util.AuthorizationException invalid null input com.okta.oidc.clients.SyncAuthClientImpl.signIn (SyncAuthClientImpl.java:115) com.okta.oidc.clients.AuthClientImpl.lambda$signIn$2 (AuthClientImpl.java:69) com.okta.oidc.clients.-$$Lambda$AuthClientImpl$bd2LRazdR1NgrGeRE1VrsI8D_uI.run (-.java:8) java.util.concurrent.Executors$RunnableAdapter.call (Executors.java:457) java.lang.Thread.run (Thread.java:764) Caused by java.lang.NullPointerException invalid null input java.security.KeyStore$PrivateKeyEntry. (KeyStore.java:559) java.security.KeyStore.getEntry (KeyStore.java:1621) com.okta.oidc.storage.security.BaseEncryptionManager.initEncodeCipher (BaseEncryptionManager.java:229) com.okta.oidc.storage.security.BaseEncryptionManager.initCipher (BaseEncryptionManager.java:189) com.okta.oidc.storage.security.BaseEncryptionManager.encrypt (BaseEncryptionManager.java:264) com.okta.oidc.storage.security.DefaultEncryptionManager.encrypt (DefaultEncryptionManager.java:46) com.okta.oidc.storage.OktaRepository.getEncrypted (OktaRepository.java:201) com.okta.oidc.storage.OktaRepository.save (OktaRepository.java:81) com.okta.oidc.OktaState.save (OktaState.java:70) com.okta.oidc.clients.AuthAPI.obtainNewConfiguration (AuthAPI.java:106) com.okta.oidc.clients.SyncAuthClientImpl.signIn (SyncAuthClientImpl.java:80) com.okta.oidc.clients.AuthClientImpl.lambda$signIn$2 (AuthClientImpl.java:69) com.okta.oidc.clients.-$$Lambda$AuthClientImpl$bd2LRazdR1NgrGeRE1VrsI8D_uI.run (-.java:8) java.util.concurrent.Executors$RunnableAdapter.call (Executors.java:457) java.lang.Thread.run (Thread.java:764)

jamesholmes-lfs commented 3 years ago

Other projects are having similar issues with Samsung:

https://github.com/adorsys/secure-storage-android/issues/86 https://github.com/oblador/react-native-keychain/issues/403

JayNewstrom commented 2 years ago

We've made a lot of fixes for crypto issues in our new SDK. Please check it out here: https://github.com/okta/okta-mobile-kotlin

JayNewstrom commented 2 years ago

We've made some small improvements in our existing implementation, and you can also implement storage in the app layer as described here: https://github.com/okta/samples-android/pull/67