Skip to content
This repository has been archived by the owner on Jun 20, 2023. It is now read-only.

Commit

Permalink
Fix automatic data reset on for users affected by encryption issues (…
Browse files Browse the repository at this point in the history
…EXPOSUREAPP-3313, EXPOSUREAPP-3335) (#1433)

* I overlooked that `KeyException` is a subclass of `GeneralSecurityException`.
This causes us to continue with `KeyException` which then fails the message matching test.
As the underlying Tink code throws the GeneralSecurityException first, we check the type of the last exception.

* Add additional specific test cases for nested exceptions.

* The dialog shouldn't automatically dismiss when the user clicks details.
They may not have read the dialog message completely or want to re-read it.
  • Loading branch information
d4rken authored Oct 21, 2020
1 parent 7be28dd commit 4a6e439
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class MainFragment : Fragment(R.layout.fragment_main) {
if (errorResetTool.isResetNoticeToBeShown) {
RecoveryByResetDialogFactory(this).showDialog(
detailsLink = R.string.errors_generic_text_catastrophic_error_encryption_failure,
onDismiss = {
onPositive = {
errorResetTool.isResetNoticeToBeShown = false
}
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,20 @@ class RecoveryByResetDialogFactory(private val fragment: Fragment) {

fun showDialog(
@StringRes detailsLink: Int,
onDismiss: () -> Unit
onPositive: () -> Unit
) {
AlertDialog.Builder(context)
val dialog = AlertDialog.Builder(context)
.setTitle(R.string.errors_generic_headline)
.setMessage(R.string.errors_generic_text_catastrophic_error_recovery_via_reset)
.setCancelable(false)
.setOnDismissListener { onDismiss() }
.setNeutralButton(R.string.errors_generic_button_negative) { _, _ ->
ExternalActionHelper.openUrl(fragment, context.getString(detailsLink))
.setNeutralButton(R.string.errors_generic_button_negative, null)
.setPositiveButton(R.string.errors_generic_button_positive) { _, _ ->
onPositive()
}
.setPositiveButton(R.string.errors_generic_button_positive) { _, _ -> }
.show()
.create()
dialog.show()
dialog.getButton(AlertDialog.BUTTON_NEUTRAL)?.setOnClickListener {
ExternalActionHelper.openUrl(fragment, context.getString(detailsLink))
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ class EncryptionErrorResetTool @Inject constructor(
}
isResetWindowConsumed = true

val keyException = error.causes().singleOrNull { it is GeneralSecurityException }
if (keyException == null) {
val keyException = error.causes().lastOrNull()
if (keyException == null || keyException !is GeneralSecurityException) {
Timber.v("Error has no GeneralSecurityException as cause -> no reset.")
return false
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ import org.junit.jupiter.api.Test
import testhelpers.BaseIOTest
import testhelpers.preferences.MockSharedPreferences
import java.io.File
import java.io.IOException
import java.security.GeneralSecurityException
import java.security.KeyException
import java.security.KeyStoreException

class EncryptionResetToolTest : BaseIOTest() {
Expand Down Expand Up @@ -169,6 +171,86 @@ class EncryptionResetToolTest : BaseIOTest() {
}
}

@Test
fun `nested exception may have the same base exception type, ie GeneralSecurityException`() {
// https://github.com/corona-warn-app/cwa-app-android/issues/642#issuecomment-712188157
createMockFiles()

createInstance().tryResetIfNecessary(
KeyException( // subclass of GeneralSecurityException
"Permantly failed to instantiate encrypted preferences",
SecurityException(
"Could not decrypt key. decryption failed",
GeneralSecurityException("decryption failed")
)
)
) shouldBe true

encryptedPrefsFile.exists() shouldBe false
encryptedDatabaseFile.exists() shouldBe false

mockPreferences.dataMapPeek.apply {
this["ea1851.reset.performedAt"] shouldNotBe null
this["ea1851.reset.windowconsumed"] shouldBe true
this["ea1851.reset.shownotice"] shouldBe true
}
}

@Test
fun `exception check does not care about the first exception type`() {
createMockFiles()

createInstance().tryResetIfNecessary(
CwaSecurityException(
KeyException( // subclass of GeneralSecurityException
"Permantly failed to instantiate encrypted preferences",
SecurityException(
"Could not decrypt key. decryption failed",
GeneralSecurityException("decryption failed")
)
)
)
) shouldBe true

encryptedPrefsFile.exists() shouldBe false
encryptedDatabaseFile.exists() shouldBe false

mockPreferences.dataMapPeek.apply {
this["ea1851.reset.performedAt"] shouldNotBe null
this["ea1851.reset.windowconsumed"] shouldBe true
this["ea1851.reset.shownotice"] shouldBe true
}
}

@Test
fun `exception check DOES care about the most nested exception`() {
createMockFiles()

createInstance().tryResetIfNecessary(
CwaSecurityException(
KeyException( // subclass of GeneralSecurityException
"Permantly failed to instantiate encrypted preferences",
SecurityException(
"Could not decrypt key. decryption failed",
GeneralSecurityException(
"decryption failed",
IOException("I am unexpeted")
)
)
)
)
) shouldBe false

encryptedPrefsFile.exists() shouldBe true
encryptedDatabaseFile.exists() shouldBe true

mockPreferences.dataMapPeek.apply {
this["ea1851.reset.performedAt"] shouldBe null
this["ea1851.reset.windowconsumed"] shouldBe true
this["ea1851.reset.shownotice"] shouldBe null
}
}

@Test
fun `we want only a specific type of GeneralSecurityException`() {
createMockFiles()
Expand Down

0 comments on commit 4a6e439

Please sign in to comment.